Harden the request path, scan and error recovery

17 files changed, 507 insertions, 190 deletions

diff --git a/source/cache.c b/source/cache.c
index 5c1c808..0c86210 100644
--- a/source/cache.c
+++ b/source/cache.c
@@ -39,6 +39,7 @@ __attribute__((format (printf,1,2)))
 static void log_error(const char *format, ...)
 {
 	va_list args;
+
 	va_start(args, format);
 	vfprintf(stderr, format, args);
 	va_end(args);
@@ -108,6 +109,7 @@ static int key_fits_slot(const char *key)
 static int close_slot(struct cache_slot *slot)
 {
 	int err = 0;
+
 	if (slot->cache_fd > 0) {
 		if (close(slot->cache_fd))
 			err = errno;
@@ -169,7 +171,7 @@ static int serve_slot(struct cache_slot *slot)
 
 	err = print_slot(slot);
 	if (err)
-		log_error("[cgit] Error printing slot %s: %s (%d)\n", slot->path, strerror(err), err);
+		log_error("[cgit] Unable to send slot %s: %s (%d)\n", slot->path, strerror(err), err);
 	return err;
 }
 
@@ -200,6 +202,7 @@ static int is_modified(struct cache_slot *slot)
 static int close_lock(struct cache_slot *slot)
 {
 	int err = 0;
+
 	if (slot->lock_fd > 0) {
 		if (close(slot->lock_fd))
 			err = errno;
@@ -278,23 +281,40 @@ static int unlock_slot(struct cache_slot *slot, int replace_old_slot)
 	return 0;
 }
 
-// Only one slot is ever being filled at a time, so a single pointer is enough
-// for cache_abandon_fill to find its way back to the client.
+// Only one slot is ever being filled at a time, so cache_abandon_fill finds
+// it here.
 static struct cache_slot *slot_being_filled;
 
-void cache_abandon_fill(void)
+/*
+ * Copies the page the lock file holds so far to stdout, which by then is the
+ * client again.
+ */
+static void replay_lock_file(struct cache_slot *slot)
+{
+	off_t off = slot->keylen + 1;
+	ssize_t got;
+
+	if (lseek(slot->lock_fd, off, SEEK_SET) != off)
+		return;
+	while ((got = xread(slot->lock_fd, slot->buf, sizeof(slot->buf))) > 0) {
+		if (write_in_full(STDOUT_FILENO, slot->buf, got) < 0)
+			return;
+	}
+}
+
+int cache_abandon_fill(void)
 {
 	struct cache_slot *slot = slot_being_filled;
 
 	if (!slot)
-		return;
+		return 0;
 	slot_being_filled = NULL;
 	slot->abandoned = 1;
 
-	// The page is sitting in html.c's buffer and in stdio's. Empty both
-	// while stdout still points at the lock file so the half rendered
-	// page never reaches the client.
-	html_flush();
+	// What git wrote through stdio joins the lock file first, then the
+	// file is played back so the visitor gets the page up to the point it
+	// stopped. The buffer in html.c is the caller's to flush, since a
+	// caller that could not write it must not try again here.
 	fflush(stdout);
 
 	if (slot->saved_stdout >= 0) {
@@ -302,11 +322,15 @@ void cache_abandon_fill(void)
 		close(slot->saved_stdout);
 		slot->saved_stdout = -1;
 	}
+	replay_lock_file(slot);
 	unlink(slot->lock_path);
+	return 1;
 }
 
-// The page is served from the descriptor either way, so a failed rename only
-// costs the next request a render.
+/*
+ * The page is served from the descriptor either way, so a failed rename only
+ * costs the next request a render.
+ */
 static void publish_slot(struct cache_slot *slot)
 {
 	int err = unlock_slot(slot, 1);
@@ -403,20 +427,21 @@ static int process_slot(struct cache_slot *slot)
 	// and two popular pages sharing one slot evict each other on every
 	// alternating visit.
 	if (!err)
-		log_error("[cgit] Cache slot %s holds a different key, consider a larger cache-size\n", slot->path);
+		log_error("[cgit] Cache slot %s holds a different key, consider a larger cache-size\n",
+			slot->path);
 
 	// If any part of creating a slot fails the page is still rendered
 	// straight to the client and the caller is told the request succeeded,
 	// because it did.
 	close_slot(slot);
 	if ((err = lock_slot(slot)) != 0) {
-		log_error("[cgit] Error locking slot %s: %s (%d)\n", slot->lock_path, strerror(err), err);
+		log_error("[cgit] Unable to lock slot %s: %s (%d)\n", slot->lock_path, strerror(err), err);
 		slot->fn();
 		return 0;
 	}
 
 	if ((err = fill_slot(slot)) != 0) {
-		log_error("[cgit] Error filling slot %s: %s (%d)\n", slot->lock_path, strerror(err), err);
+		log_error("[cgit] Unable to fill slot %s: %s (%d)\n", slot->lock_path, strerror(err), err);
 		unlock_slot(slot, 0);
 		close_lock(slot);
 		// Rendering again is only right when nothing was delivered,
@@ -441,14 +466,16 @@ static int process_slot(struct cache_slot *slot)
 	return err;
 }
 
-// The result lives in a static buffer and is only good until the next call.
-static char *format_time(const char *format, time_t when)
+/*
+ * The result lives in a static buffer and is only good until the next call.
+ */
+static const char *format_time(const char *format, time_t when)
 {
 	static char buf[64];
 	struct tm tm;
 
 	if (!when)
-		return NULL;
+		return "-";
 	gmtime_r(&when, &tm);
 	strftime(buf, sizeof(buf) - 1, format, &tm);
 	return buf;
@@ -529,8 +556,8 @@ int cache_ls(const char *path)
 	struct dirent *ent;
 	int err = 0;
 	// A NULL key leaves open_slot with nothing to compare against, so
-	// every slot it opens is simply read.
-	struct cache_slot slot = { NULL };
+	// every slot it opens is read.
+	struct cache_slot slot = { 0 };
 	struct strbuf slot_path = STRBUF_INIT;
 	size_t prefixlen;
 	char *nul;
@@ -543,20 +570,20 @@ int cache_ls(const char *path)
 	dir = opendir(path);
 	if (!dir) {
 		err = errno;
-		log_error("[cgit] Error opening %s: %s (%d)\n", path, strerror(err), err);
+		log_error("[cgit] Unable to open %s: %s (%d)\n", path, strerror(err), err);
 		return err;
 	}
 	strbuf_addstr(&slot_path, path);
 	strbuf_ensure_end(&slot_path, '/');
 	prefixlen = slot_path.len;
-	while ((ent = readdir(dir)) != NULL) {
+	while ((ent = readdir(dir))) {
 		if (strlen(ent->d_name) != SLOT_NAME_LEN)
 			continue;
 		strbuf_setlen(&slot_path, prefixlen);
 		strbuf_addstr(&slot_path, ent->d_name);
 		slot.path = slot_path.buf;
 		if ((err = open_slot(&slot)) != 0) {
-			log_error("[cgit] Error opening %s: %s (%d)\n", slot_path.buf, strerror(err), err);
+			log_error("[cgit] Unable to open %s: %s (%d)\n", slot_path.buf, strerror(err), err);
 			close_slot(&slot);
 			continue;
 		}
diff --git a/source/cache.h b/source/cache.h
index a653108..bae283c 100644
--- a/source/cache.h
+++ b/source/cache.h
@@ -19,18 +19,20 @@ typedef void (*cache_fill_fn)(void);
  */
 extern int cache_process(int size, const char *path, const char *key, int ttl, cache_fill_fn fn);
 
-// Write one line per cache slot to stdout, giving its path, modification
-// time, size and key.
+/*
+ * Write one line per cache slot to stdout, giving its path, modification time,
+ * size and key.
+ */
 extern int cache_ls(const char *path);
 
 /*
- * Give up on the slot being filled, discarding what has been rendered into it
- * and putting stdout back on the client. Anything that ends a request part way
- * through rendering has to call this before it writes what the visitor should
- * see, because until then stdout is the cache file and the visitor is on
- * course to receive nothing at all. Does nothing when no slot is being filled.
+ * Give up on the slot being filled and put stdout back on the client, after
+ * sending what was rendered into the slot so far. Anything that ends a request
+ * part way through rendering has to call this before it writes what the
+ * visitor should see, because until then stdout is the cache file. Returns
+ * non-zero when a fill was abandoned and zero when none was in progress.
  */
-extern void cache_abandon_fill(void);
+extern int cache_abandon_fill(void);
 
 extern unsigned long cache_hash_str(const char *str);
 
diff --git a/source/cgit.c b/source/cgit.c
index 5b26137..00ce97f 100644
--- a/source/cgit.c
+++ b/source/cgit.c
@@ -46,10 +46,8 @@
 // a snapshots mask can carry.
 #define ALL_SNAPSHOT_FORMATS 0xFF
 
-/*
- * The first branch found is the fallback, so a repository whose default branch
- * does not exist still has something to show.
- */
+// The first branch found is the fallback, so a repository whose default branch
+// does not exist still has something to show.
 struct refmatch {
 	char *wanted;
 	char *first;
@@ -76,13 +74,16 @@ static void isolate_git_environment(void)
 /*
  * Turn a die from anywhere inside git into a rendered page, since a CGI that
  * produced no output leaves the visitor with whatever the web server makes of
- * it.
+ * it. Git's message names files and objects on the server, so it goes to the
+ * log and the visitor gets a fixed page.
  */
 static NORETURN void die_routine(const char *msg, va_list params)
 {
-	// The error page abandons any cache fill in progress, so the message
-	// reaches the visitor rather than the cache file stdout points at.
-	cgit_vprint_error_page(400, "Bad Request", msg, params);
+	fputs("[cgit] ", stderr);
+	vfprintf(stderr, msg, params);
+	fputc('\n', stderr);
+	cgit_abort_filters();
+	cgit_print_error_page(500, "Internal Server Error", "Unable to complete the request");
 	exit(0);
 }
 
@@ -154,7 +155,8 @@ static void prepare_context(void)
 	ctx.env.server_port = getenv("SERVER_PORT");
 	ctx.env.http_cookie = getenv("HTTP_COOKIE");
 	ctx.env.http_referer = getenv("HTTP_REFERER");
-	ctx.env.content_length = getenv("CONTENT_LENGTH") ? strtoul(getenv("CONTENT_LENGTH"), NULL, 10) : 0;
+	ctx.env.content_length =
+		getenv("CONTENT_LENGTH") ? strtoul(getenv("CONTENT_LENGTH"), NULL, 10) : 0;
 	ctx.env.authenticated = 0;
 	ctx.page.mimetype = "text/html";
 	ctx.page.charset = PAGE_ENCODING;
@@ -189,6 +191,7 @@ static void print_version(void)
 static int cmp_repos(const void *a, const void *b)
 {
 	const struct cgit_repo *repo_a = a, *repo_b = b;
+
 	return strcmp(repo_a->url, repo_b->url);
 }
 
@@ -213,7 +216,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
 
 	fprintf(f, "repo.url=%s\n", repo->url);
 	fprintf(f, "repo.name=%s\n", repo->name);
-	fprintf(f, "repo.path=%s\n", repo->path);
+	if (repo->path)
+		fprintf(f, "repo.path=%s\n", repo->path);
 	if (repo->owner)
 		fprintf(f, "repo.owner=%s\n", repo->owner);
 	if (repo->desc)
@@ -318,7 +322,10 @@ static void parse_args(int argc, const char **argv)
 			ctx.qry.has_oid = 1;
 		} else if (skip_prefix(argv[i], "--ofs=", &arg)) {
 			ctx.qry.ofs = atoi(arg);
-		} else if (skip_prefix(argv[i], "--scan-tree=", &arg) || skip_prefix(argv[i], "--scan-path=", &arg)) {
+		} else if (
+			skip_prefix(argv[i], "--scan-tree=", &arg) ||
+			skip_prefix(argv[i], "--scan-path=", &arg)
+		) {
 			// A repository's snapshots setting is masked with the
 			// global one, and cgitrc has not been read here, so an
 			// empty mask would discard what the repository set.
@@ -358,7 +365,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 	fd = open(locked_rc.buf, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
 	if (fd == -1) {
 		err = errno;
-		fprintf(stderr, "[cgit] Error opening %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
+		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		goto out;
 	}
 	if (fcntl(fd, F_SETLK, &lock) < 0) {
@@ -368,7 +375,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 		// on every request and does deserve one.
 		err = errno;
 		if (err != EACCES && err != EAGAIN)
-			fprintf(stderr, "[cgit] Error locking %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
+			fprintf(stderr, "[cgit] Unable to lock %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		close(fd);
 		goto out;
 	}
@@ -390,7 +397,7 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 	// start from empty now that nobody else can be writing it.
 	if (ftruncate(fd, 0) < 0 || !(f = fdopen(fd, "w"))) {
 		err = errno;
-		fprintf(stderr, "[cgit] Error writing %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
+		fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		unlink(locked_rc.buf);
 		close(fd);
 		goto out;
@@ -406,14 +413,14 @@ static int generate_cached_repolist(const char *path, const char *cached_rc)
 	// that stops wherever the buffer happened to end.
 	if (fflush(f) || ferror(f)) {
 		err = errno;
-		fprintf(stderr, "[cgit] Error writing %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
+		fprintf(stderr, "[cgit] Unable to write %s: %s (%d)\n", locked_rc.buf, strerror(err), err);
 		unlink(locked_rc.buf);
 		fclose(f);
 		goto out;
 	}
 	if (rename(locked_rc.buf, cached_rc)) {
 		err = errno;
-		fprintf(stderr, "[cgit] Error renaming %s to %s: %s (%d)\n",
+		fprintf(stderr, "[cgit] Unable to rename %s to %s: %s (%d)\n",
 			locked_rc.buf, cached_rc, strerror(err), err);
 		unlink(locked_rc.buf);
 	}
@@ -425,7 +432,9 @@ out:
 	return err;
 }
 
-// A cached repolist is itself a config file, so these two call each other.
+/*
+ * A cached repolist is itself a config file, so these two call each other.
+ */
 static void apply_config(const char *name, const char *value);
 
 static void process_cached_repolist(const char *path)
@@ -539,8 +548,8 @@ static void apply_config(const char *name, const char *value)
 		ctx.cfg.enable_header = atoi(value);
 	else if (!strcmp(name, "snapshots"))
 		ctx.cfg.snapshots = cgit_parse_snapshots_mask(value);
-	else if (!strcmp(name, "trust-scan-filters"))
-		ctx.cfg.trust_scan_filters = atoi(value);
+	else if (!strcmp(name, "trust-scan-config"))
+		ctx.cfg.trust_scan_config = atoi(value);
 	else if (!strcmp(name, "enable-follow-links"))
 		ctx.cfg.enable_follow_links = atoi(value);
 	else if (!strcmp(name, "enable-http-clone"))
@@ -644,6 +653,9 @@ static void apply_config(const char *name, const char *value)
 			scan_projects(cgit_expand_macros(value), ctx.cfg.project_list);
 		else
 			scan_tree(cgit_expand_macros(value));
+		// The scan grows the repository array, so a record taken before
+		// it may have moved, and a repo key after it belongs to nothing.
+		ctx.repo = NULL;
 	} else if (!strcmp(name, "scan-hidden-path"))
 		ctx.cfg.scan_hidden_path = atoi(value);
 	else if (!strcmp(name, "section-from-path"))
@@ -721,7 +733,7 @@ static void apply_query_param(const char *name, const char *value)
 	if (!value)
 		value = "";
 
-	if (!strcmp(name,"r")) {
+	if (!strcmp(name, "r")) {
 		ctx.qry.repo = xstrdup(value);
 		ctx.repo = cgit_get_repoinfo(value);
 	} else if (!strcmp(name, "p")) {
@@ -814,10 +826,12 @@ static void authenticate_post(void)
 	len = ctx.env.content_length;
 	if (len > MAX_AUTHENTICATION_POST_BYTES)
 		len = MAX_AUTHENTICATION_POST_BYTES;
-	if ((got = read(STDIN_FILENO, buffer, len)) < 0)
-		die_errno("Could not read POST from stdin");
-	if (write(STDOUT_FILENO, buffer, got) < 0)
-		die_errno("Could not write POST to stdout");
+	// The body can arrive in more than one write, so read until it is
+	// all there or the server closes the stream.
+	if ((got = read_in_full(STDIN_FILENO, buffer, len)) < 0)
+		die_errno("Unable to read the POST body");
+	if (write_in_full(STDOUT_FILENO, buffer, got) < 0)
+		die_errno("Unable to pass the POST body to the auth filter");
 	cgit_close_filter(ctx.cfg.auth_filter);
 	exit(0);
 }
@@ -856,13 +870,42 @@ static int is_full_oid(const char *rev)
 	if (len != GIT_SHA1_HEXSZ && len != GIT_SHA256_HEXSZ)
 		return 0;
 	for (; *rev; rev++) {
-		if (!isxdigit(*rev))
+		if (!isxdigit((unsigned char)*rev))
 			return 0;
 	}
 	return 1;
 }
 
-// Every cache-*-ttl setting is written in minutes, and so is this.
+/*
+ * Only these pages render the same bytes for the same id for ever. A log or
+ * refs page named with an id still lists branches and tags, which move.
+ */
+static int page_is_static(void)
+{
+	static const char *const pages[] = {
+		"blame",
+		"blob",
+		"commit",
+		"diff",
+		"patch",
+		"plain",
+		"rawdiff",
+		"snapshot",
+		"tag",
+		"tree",
+	};
+	size_t i;
+
+	for (i = 0; i < ARRAY_SIZE(pages); i++) {
+		if (!strcmp(ctx.qry.page, pages[i]))
+			return 1;
+	}
+	return 0;
+}
+
+/*
+ * Every cache-*-ttl setting is written in minutes, and so is this.
+ */
 static int calc_ttl(void)
 {
 	if (!ctx.repo)
@@ -879,6 +922,7 @@ static int calc_ttl(void)
 	// page to rebuild.
 	if (
 		ctx.qry.has_oid								&&
+		page_is_static()							&&
 		(!ctx.qry.oid || is_full_oid(ctx.qry.oid))	&&
 		(!ctx.qry.oid2 || is_full_oid(ctx.qry.oid2))
 	)
@@ -917,14 +961,18 @@ static void build_cache_key(struct strbuf *key)
 	free(hosturl);
 }
 
-// Returning non-zero ends git's walk, so the search stops at the first hit.
+/*
+ * Returning non-zero ends git's walk, so the search stops at the first hit.
+ */
 static int find_current_ref(const struct reference *ref, void *data)
 {
 	struct refmatch *match = data;
 
 	if (!strcmp(ref->name, match->wanted))
 		match->found = 1;
-	if (!match->first)
+	// The fallback has to pass the check every request makes on its head,
+	// or one branch named with a leading dash takes the repository offline.
+	if (!match->first && ref->name[0] != '-')
 		match->first = xstrdup(ref->name);
 	return match->found;
 }
@@ -996,14 +1044,18 @@ static void parse_readme(const char *readme, char **filename, char **ref, struct
 static void choose_readme(struct cgit_repo *repo)
 {
 	int found;
-	char *filename, *ref;
+	char *filename = NULL, *ref = NULL;
+	struct string_list *list;
 	struct string_list_item *entry;
 
-	if (!repo->readme.nr)
+	// A repository without readme settings of its own follows the global
+	// ones, which it must not free.
+	list = repo->readme.nr ? &repo->readme : &ctx.cfg.readme;
+	if (!list->nr)
 		return;
 
 	found = 0;
-	for_each_string_list_item(entry, &repo->readme) {
+	for_each_string_list_item(entry, list) {
 		parse_readme(entry->string, &filename, &ref, repo);
 		if (!filename) {
 			free(ref);
@@ -1028,11 +1080,19 @@ static void choose_readme(struct cgit_repo *repo)
 		string_list_append(&repo->readme, filename)->util = ref;
 }
 
-static void prepare_repo_env(int *nongit)
+static void prepare_repo_env(int *nongit, int *err)
 {
+	// A repository configured without a path has nothing to open.
+	if (!ctx.repo->path) {
+		*nongit = 1;
+		*err = 0;
+		return;
+	}
 	setenv("GIT_DIR", ctx.repo->path, 1);
 
+	errno = 0;
 	setup_git_directory_gently(the_repository, nongit);
+	*err = errno;
 	// Notes come out of the object store, which a repository that failed
 	// to open has none of.
 	if (!*nongit)
@@ -1043,14 +1103,12 @@ static void prepare_repo_env(int *nongit)
  * Returns non-zero once it has written a complete response of its own, in
  * which case the caller must not render a page over the top of it.
  */
-static int prepare_repo_cmd(int nongit)
+static int prepare_repo_cmd(int nongit, int err, int clone)
 {
 	struct object_id oid;
-	int err;
 
 	if (nongit) {
 		const char *name = ctx.repo->name;
-		err = errno;
 		ctx.page.title = cgit_fmtalloc("%s - %s", ctx.cfg.root_title, "config error");
 		ctx.repo = NULL;
 		cgit_print_error_page(
@@ -1070,6 +1128,12 @@ static int prepare_repo_cmd(int nongit)
 	}
 
 	if (!ctx.qry.head) {
+		// The dumb transport serves refs and objects off the disk and
+		// asks nothing of the head, and an empty repository clones.
+		if (clone) {
+			cgit_prepare_repo_env(ctx.repo);
+			return 0;
+		}
 		ctx.empty_repo = 1;
 		// Before the document starts, since the <head> carries
 		// <link rel='vcs-git'> and those clone urls expand macros such
@@ -1083,13 +1147,22 @@ static int prepare_repo_cmd(int nongit)
 		return 1;
 	}
 
-	if (repo_get_oid(the_repository, ctx.qry.head, &oid)) {
+	// Every revision the request names is checked before git sees it,
+	// see cgit_valid_rev, and the head has to resolve as well.
+	if (!cgit_valid_rev(ctx.qry.head) || repo_get_oid(the_repository, ctx.qry.head, &oid)) {
 		char *old_head = ctx.qry.head;
 		ctx.qry.head = xstrdup(ctx.repo->defbranch);
 		cgit_print_error_page(404, "Not Found", "Invalid branch: %s", old_head);
 		free(old_head);
 		return 1;
 	}
+	if (
+		(ctx.qry.oid && !cgit_valid_rev(ctx.qry.oid))	||
+		(ctx.qry.oid2 && !cgit_valid_rev(ctx.qry.oid2))
+	) {
+		cgit_print_error_page(400, "Bad Request", "Invalid revision");
+		return 1;
+	}
 	string_list_sort(&ctx.repo->submodules);
 	cgit_prepare_repo_env(ctx.repo);
 	choose_readme(ctx.repo);
@@ -1099,7 +1172,7 @@ static int prepare_repo_cmd(int nongit)
 static void process_request(void)
 {
 	const struct cgit_cmd *cmd;
-	int nongit = 0;
+	int nongit = 0, err = 0;
 
 	// An unauthenticated request is answered with the filter's own body
 	// whatever page it asked for.
@@ -1115,16 +1188,14 @@ static void process_request(void)
 	}
 
 	if (ctx.repo)
-		prepare_repo_env(&nongit);
+		prepare_repo_env(&nongit, &err);
 
 	cmd = cgit_get_cmd();
-	if (!cmd) {
-		ctx.page.title = "cgit error";
-		cgit_print_error_page(404, "Not Found", "Invalid request");
-		return;
-	}
-
-	if (!ctx.cfg.enable_http_clone && cmd->is_clone) {
+	if (!cmd || (!ctx.cfg.enable_http_clone && cmd->is_clone)) {
+		// The error page carries the repository's own header, whose
+		// branch switcher needs the head resolved first.
+		if (ctx.repo && prepare_repo_cmd(nongit, err, 0))
+			return;
 		ctx.page.title = "cgit error";
 		cgit_print_error_page(404, "Not Found", "Invalid request");
 		return;
@@ -1137,7 +1208,7 @@ static void process_request(void)
 
 	ctx.qry.vpath = cmd->want_vpath ? ctx.qry.path : NULL;
 
-	if (ctx.repo && prepare_repo_cmd(nongit))
+	if (ctx.repo && prepare_repo_cmd(nongit, err, cmd->is_clone))
 		return;
 
 	cmd->fn();
@@ -1205,13 +1276,11 @@ void cgit_repo_config(struct cgit_repo *repo, const char *name, const char *valu
 		repo->section = cgit_strdup_first_line(value);
 	else if (!strcmp(name, "snapshot-prefix"))
 		repo->snapshot_prefix = cgit_strdup_first_line(value);
-	else if (!strcmp(name, "readme") && value != NULL) {
-		if (repo->readme.items == ctx.cfg.readme.items)
-			memset(&repo->readme, 0, sizeof(repo->readme));
+	else if (!strcmp(name, "readme"))
 		string_list_append(&repo->readme, cgit_strdup_first_line(value));
-	} else if (!strcmp(name, "logo") && value != NULL)
+	else if (!strcmp(name, "logo"))
 		repo->logo = cgit_strdup_first_line(value);
-	else if (!strcmp(name, "logo-link") && value != NULL)
+	else if (!strcmp(name, "logo-link"))
 		repo->logo_link = cgit_strdup_first_line(value);
 	else if (!strcmp(name, "hide"))
 		repo->hide = atoi(value);
@@ -1238,6 +1307,9 @@ int cmd_main(int argc, const char **argv)
 	int err, ttl;
 
 	isolate_git_environment();
+	// Ignored, a filter that exits early makes the write fail with EPIPE
+	// and the error page reach the visitor, instead of ending cgit.
+	signal(SIGPIPE, SIG_IGN);
 	cgit_init_filters();
 	atexit(cgit_cleanup_filters);
 	// Registered second so it runs first, since exit is reached from error
@@ -1271,8 +1343,9 @@ int cmd_main(int argc, const char **argv)
 			char *path_and_query = cgit_fmtalloc("%s?%s", path, ctx.qry.raw);
 			free(ctx.qry.raw);
 			ctx.qry.raw = path_and_query;
-		} else
+		} else {
 			ctx.qry.raw = xstrdup(ctx.qry.url);
+		}
 		cgit_parse_url(ctx.qry.url);
 	}
 
diff --git a/source/cgit.h b/source/cgit.h
index 72c0844..163cd4d 100644
--- a/source/cgit.h
+++ b/source/cgit.h
@@ -38,6 +38,7 @@
 #include <string-list.h>
 #include <strvec.h>
 #include <tag.h>
+#include <tree-walk.h>
 #include <tree.h>
 #include <url.h>
 #include <utf8.h>
@@ -58,6 +59,13 @@
 // A double, because a twelfth of a year is not a whole number of seconds.
 #define SECONDS_PER_MONTH  (SECONDS_PER_YEAR / 12.0)
 
+// The number of context lines git itself defaults to.
+#define DEFAULT_DIFF_CONTEXT 3
+
+// A tab in rendered source runs on to the next multiple of this, which is the
+// width a browser picks on its own since the stylesheet sets no tab-size.
+#define TAB_WIDTH 8
+
 typedef enum {
 	DIFF_UNIFIED, DIFF_SSDIFF, DIFF_STATONLY
 } diff_type;
@@ -168,7 +176,7 @@ struct cgit_query {
 	char *path;
 	char *url;
 	char *period;
-	int   ofs;
+	int ofs;
 	int nohead;
 	char *sort;
 	int showmsg;
@@ -203,7 +211,8 @@ struct cgit_config {
 	char *script_name;
 	char *section;
 	char *repository_sort;
-	char *virtual_root;	// Always ends with a slash.
+	// Always ends with a slash.
+	char *virtual_root;
 	char *strict_export;
 	struct date_mode date_mode;
 	int cache_size;
@@ -216,7 +225,7 @@ struct cgit_config {
 	int cache_snapshot_ttl;
 	int case_sensitive_sort;
 	int embedded;
-	int trust_scan_filters;
+	int trust_scan_config;
 	int enable_follow_links;
 	int enable_gitmodules_links;
 	int enable_header;
@@ -281,6 +290,8 @@ struct cgit_page {
 	int status;
 	const char *statusmsg;
 	int untrusted;
+	// Set once the status line is out, when an error can only join the body.
+	int headers_sent;
 };
 
 struct cgit_environment {
diff --git a/source/cgit.mk b/source/cgit.mk
index 1199278..4109930 100644
--- a/source/cgit.mk
+++ b/source/cgit.mk
@@ -27,18 +27,14 @@ CGIT_BUILD = $(CGIT_ROOT)/$(BUILDDIR)
 # The CGIT_ values used below are exported by the top level Makefile.
 $(CGIT_BUILD)/VERSION: force-version
 	@mkdir -p $(CGIT_BUILD)/
-	@cd $(CGIT_ROOT) && '$(SHELL_PATH_SQ)' $(TOOLSDIR)/gen-version.sh "$(CGIT_VERSION)" $(BUILDDIR)/VERSION
+	@cd $(CGIT_ROOT) && \
+		'$(SHELL_PATH_SQ)' $(TOOLSDIR)/gen-version.sh "$(CGIT_VERSION)" $(BUILDDIR)/VERSION
 -include $(CGIT_BUILD)/VERSION
 .PHONY: force-version
 
-# The language the cgit sources are written in. Both GCC and Clang default to
-# this today, so pinning it changes nothing now and stops the meaning of the
-# sources drifting when a compiler moves its default on, as GCC 15 did by
-# defaulting to gnu23. The GNU dialect rather than plain c17 because git's
-# headers use GNU extensions, and because dlsym cannot be used through a
-# conforming cast. Only the cgit objects are held to this, and Git keeps
-# whatever its own build decides, which on some platforms is a different
-# standard again.
+# Pinned so a compiler moving its default, as GCC 15 did to gnu23, cannot
+# change what the sources mean. The GNU dialect because git's headers use GNU
+# extensions and dlsym needs a non-conforming cast. Git keeps its own choice.
 CGIT_STD ?= gnu17
 
 # CGIT_CFLAGS is tracked separately so that changing it does not force a rebuild
@@ -157,12 +153,14 @@ $(CGIT_BUILD)/.depend:
 $(CGIT_BUILD)/CGIT-CFLAGS: FORCE
 	@mkdir -p $(CGIT_BUILD)/
 	@FLAGS='$(subst ','\'',$(CGIT_CFLAGS))'; \
-	    if test x"$$FLAGS" != x"`cat $(CGIT_BUILD)/CGIT-CFLAGS 2>/dev/null`" ; then \
-		echo 1>&2 "    * new CGit build flags"; \
-		echo "$$FLAGS" >$(CGIT_BUILD)/CGIT-CFLAGS; \
-	    fi
+		OLD=`cat $(CGIT_BUILD)/CGIT-CFLAGS 2>/dev/null`; \
+		if test x"$$FLAGS" != x"$$OLD"; then \
+			echo 1>&2 "    * new CGit build flags"; \
+			echo "$$FLAGS" >$(CGIT_BUILD)/CGIT-CFLAGS; \
+		fi
 
-$(CGIT_OBJS): $(CGIT_BUILD)/%.o: $(CGIT_SRC)/%.c GIT-CFLAGS $(CGIT_BUILD)/CGIT-CFLAGS $(missing_dep_dirs)
+$(CGIT_OBJS): $(CGIT_BUILD)/%.o: $(CGIT_SRC)/%.c GIT-CFLAGS $(CGIT_BUILD)/CGIT-CFLAGS \
+		$(missing_dep_dirs)
 	$(QUIET_CC)$(CC) -o $@ -c $(dep_args) $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) $(CGIT_CFLAGS) $<
 
 $(CGIT_BUILD)/cgit: $(CGIT_OBJS) GIT-LDFLAGS $(GITLIBS)
diff --git a/source/cmd.c b/source/cmd.c
index 2c6406f..a19aa50 100644
--- a/source/cmd.c
+++ b/source/cmd.c
@@ -212,7 +212,7 @@ const struct cgit_cmd *cgit_get_cmd(void)
 {
 	size_t i;
 
-	if (ctx.qry.page == NULL) {
+	if (!ctx.qry.page) {
 		if (ctx.repo)
 			ctx.qry.page = "summary";
 		else
diff --git a/source/cmd.h b/source/cmd.h
index a75cdd0..f580fa1 100644
--- a/source/cmd.h
+++ b/source/cmd.h
@@ -16,7 +16,9 @@ struct cgit_cmd {
 	unsigned int want_repo:1, want_vpath:1, is_clone:1;
 };
 
-// The entry naming ctx.qry.page, or NULL when no page goes by that name.
+/*
+ * The entry naming ctx.qry.page, or NULL when no page goes by that name.
+ */
 extern const struct cgit_cmd *cgit_get_cmd(void);
 
 #endif // CGIT_CMD_H
diff --git a/source/config.c b/source/config.c
index 0ff8588..2e79791 100644
--- a/source/config.c
+++ b/source/config.c
@@ -70,7 +70,7 @@ static int read_entry(FILE *f, struct strbuf *name, struct strbuf *value)
 		}
 		if (c == EOF)
 			return 0;
-		// Dropping just the line with no equals sign keeps one typo
+		// Dropping only the line with no equals sign keeps one typo
 		// from hiding the rest of the file.
 		if (c != '=')
 			continue;
diff --git a/source/filter.c b/source/filter.c
index 1cfdde2..f5fb2d5 100644
--- a/source/filter.c
+++ b/source/filter.c
@@ -22,6 +22,9 @@
 // for a command not found.
 #define EXEC_FAILED 127
 
+// The exec filter holding stdout, for cgit_abort_filters to take it back.
+static struct cgit_exec_filter *running_exec;
+
 static int open_exec_filter(struct cgit_filter *base, va_list ap)
 {
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
@@ -31,13 +34,19 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap)
 	for (i = 0; i < filter->base.argument_count; i++)
 		filter->argv[i + 1] = va_arg(ap, char *);
 
-	filter->old_stdout = cgit_die_unless_positive(dup(STDOUT_FILENO), "Unable to duplicate STDOUT");
+	filter->old_stdout = cgit_die_unless_positive(dup(STDOUT_FILENO), "Unable to duplicate stdout");
+	// The child keeps the page's stdout, but not the descriptor the parent
+	// needs it back from.
+	fcntl(filter->old_stdout, F_SETFD, FD_CLOEXEC);
 	cgit_die_unless_zero(pipe(pipefd), "Unable to create pipe to subprocess");
 	filter->pid = cgit_die_unless_non_negative(fork(), "Unable to create subprocess");
 	if (filter->pid == 0) {
 		close(pipefd[1]);
 		if (dup2(pipefd[0], STDIN_FILENO) < 0)
 			_exit(EXEC_FAILED);
+		// cgit ignores SIGPIPE and the disposition survives exec, so a
+		// filter that writes to a closed pipe would otherwise carry on.
+		signal(SIGPIPE, SIG_DFL);
 		execvp(filter->cmd, filter->argv);
 		// The child shares the parent's page buffer, cache lock and exit
 		// handlers, so it must not die through them.
@@ -45,11 +54,9 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap)
 		_exit(EXEC_FAILED);
 	}
 	close(pipefd[0]);
-	// The child keeps the page's stdout, but not the descriptor the parent
-	// needs it back from.
-	fcntl(filter->old_stdout, F_SETFD, FD_CLOEXEC);
-	cgit_die_unless_non_negative(dup2(pipefd[1], STDOUT_FILENO), "Unable to use pipe as STDOUT");
+	cgit_die_unless_non_negative(dup2(pipefd[1], STDOUT_FILENO), "Unable to use pipe as stdout");
 	close(pipefd[1]);
+	running_exec = filter;
 	return 0;
 }
 
@@ -58,7 +65,8 @@ static int close_exec_filter(struct cgit_filter *base)
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
 	int i, exit_status = 0;
 
-	cgit_die_unless_non_negative(dup2(filter->old_stdout, STDOUT_FILENO), "Unable to restore STDOUT");
+	running_exec = NULL;
+	cgit_die_unless_non_negative(dup2(filter->old_stdout, STDOUT_FILENO), "Unable to restore stdout");
 	close(filter->old_stdout);
 	if (filter->pid < 0)
 		goto done;
@@ -77,12 +85,14 @@ done:
 static void fprintf_exec_filter(struct cgit_filter *base, FILE *f, const char *prefix)
 {
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
+
 	fprintf(f, "%sexec:%s\n", prefix, filter->cmd);
 }
 
 static void cleanup_exec_filter(struct cgit_filter *base)
 {
 	struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base;
+
 	free(filter->argv);
 	filter->argv = NULL;
 	free(filter->cmd);
@@ -141,7 +151,7 @@ void cgit_init_filters(void)
 {
 	libc_write = dlsym(RTLD_NEXT, "write");
 	if (!libc_write)
-		die("Could not locate libc's write function");
+		die("Unable to find libc's write function");
 }
 
 /*
@@ -159,16 +169,16 @@ static inline void hook_write(struct cgit_filter *filter, filter_write_fn write_
 {
 	// Filters cannot nest, because there is one stdout and one hook, so a
 	// second one would strand the first.
-	assert(filter_write == NULL);
-	assert(current_write_filter == NULL);
+	assert(!filter_write);
+	assert(!current_write_filter);
 	current_write_filter = filter;
 	filter_write = write_fn;
 }
 
 static inline void unhook_write(void)
 {
-	assert(filter_write != NULL);
-	assert(current_write_filter != NULL);
+	assert(filter_write);
+	assert(current_write_filter);
 	filter_write = NULL;
 	current_write_filter = NULL;
 }
@@ -322,7 +332,7 @@ static int close_lua_filter(struct cgit_filter *base)
 	lua_getglobal(filter->lua_state, "filter_close");
 	if (lua_pcall(filter->lua_state, 0, 1, 0))
 		die_lua_error(filter);
-	ret = lua_tonumber(filter->lua_state, -1);
+	ret = (int)lua_tointeger(filter->lua_state, -1);
 	lua_pop(filter->lua_state, 1);
 
 	unhook_write();
@@ -368,6 +378,7 @@ int cgit_open_filter(struct cgit_filter *filter, ...)
 {
 	int result;
 	va_list ap;
+
 	if (!filter)
 		return 0;
 	// Whatever is still buffered belongs to the page, not to the filter
@@ -403,6 +414,7 @@ static inline void cleanup_filter(struct cgit_filter *filter)
 void cgit_cleanup_filters(void)
 {
 	int i;
+
 	cleanup_filter(ctx.cfg.about_filter);
 	cleanup_filter(ctx.cfg.commit_filter);
 	cleanup_filter(ctx.cfg.source_filter);
@@ -418,6 +430,24 @@ void cgit_cleanup_filters(void)
 	}
 }
 
+/*
+ * Take stdout back from a filter that holds it when a die lands, so the error
+ * page reaches the visitor and not the filter. What the page had buffered for
+ * the filter is dropped with it, since it was never meant to go out as it is.
+ * An exec child sees the end of its input and exits on its own.
+ */
+void cgit_abort_filters(void)
+{
+	html_discard();
+	if (running_exec) {
+		dup2(running_exec->old_stdout, STDOUT_FILENO);
+		close(running_exec->old_stdout);
+		running_exec = NULL;
+	}
+	if (filter_write)
+		unhook_write();
+}
+
 static const struct {
 	const char *prefix;
 	struct cgit_filter *(*create)(const char *cmd, int argument_count);
@@ -473,5 +503,5 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
 			return filter_specs[i].create(colon + 1, argument_count);
 	}
 
-	die("Invalid filter type: %.*s", (int) len, cmd);
+	die("Invalid filter type: %.*s", (int)len, cmd);
 }
diff --git a/source/filter.h b/source/filter.h
index fa43416..b5a478f 100644
--- a/source/filter.h
+++ b/source/filter.h
@@ -44,11 +44,19 @@ extern void cgit_exec_filter_init(struct cgit_exec_filter *filter, char *cmd,
 extern int cgit_open_filter(struct cgit_filter *filter, ...);
 extern int cgit_close_filter(struct cgit_filter *filter);
 
-// Describe the filter the way cgitrc would have configured it.
+/*
+ * Describe the filter the way cgitrc would have configured it.
+ */
 extern void cgit_fprintf_filter(struct cgit_filter *filter, FILE *f,
 				const char *prefix);
 
 extern void cgit_init_filters(void);
 extern void cgit_cleanup_filters(void);
 
+/*
+ * Take stdout back from whichever filter holds it, dropping what was buffered
+ * for that filter. For the die path only.
+ */
+extern void cgit_abort_filters(void);
+
 #endif // CGIT_FILTER_H
diff --git a/source/html.c b/source/html.c
index 5972d97..a1d244b 100644
--- a/source/html.c
+++ b/source/html.c
@@ -9,6 +9,7 @@
  */
 
 #include "cgit.h"
+#include "cache.h"
 #include "html.h"
 
 #define HTML_WRITE_BUFSIZE (64 * 1024)
@@ -58,8 +59,22 @@ static void write_out(const char *data, size_t size)
 {
 	// A blob or snapshot is well past what one write can move onto a
 	// pipe, so short writes are resumed rather than reported.
-	if (write_in_full(STDOUT_FILENO, data, size) < 0)
-		die_errno("write error on html output");
+	while (size) {
+		ssize_t written = xwrite(STDOUT_FILENO, data, size);
+
+		if (written < 0)
+			break;
+		data += written;
+		size -= written;
+	}
+	if (!size)
+		return;
+	// While a cache slot is being filled stdout is a file, and a full disk
+	// must not cost the visitor the page. Abandoning the fill puts stdout
+	// back on the client along with what the file already holds, so only
+	// the rest is written again.
+	if (!cache_abandon_fill() || write_in_full(STDOUT_FILENO, data, size) < 0)
+		die_errno("Unable to write the page");
 }
 
 /*
@@ -111,6 +126,11 @@ void html_flush(void)
 	write_out(out_buf, len);
 }
 
+void html_discard(void)
+{
+	out_len = 0;
+}
+
 void html_capture_begin(struct strbuf *sb)
 {
 	html_flush();
@@ -191,7 +211,7 @@ ssize_t html_ntxt(const char *txt, size_t len)
 	if (len > SSIZE_MAX)
 		return -1;
 
-	left = (ssize_t) len;
+	left = (ssize_t)len;
 	while (p && *p && left--) {
 		int c = *p;
 		if (c == '<' || c == '>' || c == '&') {
@@ -227,6 +247,7 @@ void html_attrf(const char *format, ...)
 void html_attr(const char *txt)
 {
 	const char *p = txt;
+
 	while (p && *p) {
 		int c = *p;
 		if (c == '<' || c == '>' || c == '\'' || c == '\"' || c == '&') {
@@ -252,6 +273,7 @@ void html_attr(const char *txt)
 void html_url_path(const char *txt)
 {
 	const char *p = txt;
+
 	while (p && *p) {
 		unsigned char c = *p;
 		// A raw ampersand or plus is legal in a URL path, but this
@@ -274,6 +296,7 @@ void html_url_path(const char *txt)
 void html_url_arg(const char *txt)
 {
 	const char *p = txt;
+
 	while (p && *p) {
 		unsigned char c = *p;
 		const char *esc = url_escape_table[c];
@@ -293,6 +316,7 @@ void html_url_arg(const char *txt)
 void html_header_arg_in_quotes(const char *txt)
 {
 	const char *p = txt;
+
 	while (p && *p) {
 		unsigned char c = *p;
 		const char *esc = NULL;
@@ -379,7 +403,7 @@ int html_include(const char *filename)
 	size_t len;
 
 	if (!(f = fopen(filename, "r"))) {
-		fprintf(stderr, "[cgit] Error including file %s: %s (%d)\n", filename, strerror(errno), errno);
+		fprintf(stderr, "[cgit] Unable to include %s: %s (%d)\n", filename, strerror(errno), errno);
 		return -1;
 	}
 	while ((len = fread(buf, 1, sizeof(buf), f)) > 0)
diff --git a/source/html.h b/source/html.h
index 410661b..46b445f 100644
--- a/source/html.h
+++ b/source/html.h
@@ -12,10 +12,9 @@
 
 #include "cgit.h"
 
-// How much of a generated run to build before handing it to html_raw. Growing
-// past this gains nothing, since html_raw gathers what it is given into a
-// buffer of its own, and a run built whole would grow with the file it came
-// from, to many times the blob's size, which max-blob-size never measures.
+// How much of a generated run to build before handing it to html_raw. A run
+// built whole would grow with the file it came from, to many times the blob's
+// size, which max-blob-size never measures.
 #define HTML_BATCH (64 * 1024)
 
 extern void html_raw(const char *txt, size_t size);
@@ -28,6 +27,11 @@ extern void html(const char *txt);
 extern void html_flush(void);
 
 /*
+ * Drop whatever page output is still buffered. For the die path only.
+ */
+extern void html_discard(void);
+
+/*
  * Collect page output into sb rather than writing it, until html_capture_end.
  * A capture must not span a filter or anything else that writes to stdout
  * directly, since that output would escape the capture.
diff --git a/source/parsing.c b/source/parsing.c
index 5fde25d..655973b 100644
--- a/source/parsing.c
+++ b/source/parsing.c
@@ -31,11 +31,9 @@ static char *substr(const char *start, const char *end)
 	return buf;
 }
 
-/*
- * The repository's mailmap, read once per request and applied to every ident
- * a page shows. Only the blob at HEAD is read, the way git reads a bare
- * repository, so a repository cannot point the reader at a file on the server.
- */
+// The repository's mailmap, read once per request and applied to every ident a
+// page shows. Only the blob at HEAD is read, the way git reads a bare
+// repository, so a repository cannot point the reader at a file on the server.
 static struct string_list mailmap = STRING_LIST_INIT_DUP;
 static int mailmap_read;
 
@@ -175,7 +173,7 @@ struct commitinfo *cgit_parse_commit(struct commit *commit)
 		return info;
 
 	if (!skip_prefix(p, "tree ", &p))
-		die("Bad commit: %s", oid_to_hex(&commit->object.oid));
+		die("Unable to parse commit %s", oid_to_hex(&commit->object.oid));
 	p += the_hash_algo->hexsz + 1;
 
 	while (skip_prefix(p, "parent ", &p))
@@ -187,7 +185,8 @@ struct commitinfo *cgit_parse_commit(struct commit *commit)
 	}
 
 	if (p && skip_prefix(p, "committer ", &p)) {
-		parse_user(p, &info->committer, &info->committer_email, &info->committer_date, &info->committer_tz);
+		parse_user(p, &info->committer, &info->committer_email, &info->committer_date,
+			&info->committer_tz);
 		p = next_header_line(p);
 	}
 
@@ -247,9 +246,8 @@ struct taginfo *cgit_parse_tag(struct tag *tag)
 	info = xcalloc(1, sizeof(struct taginfo));
 
 	for (p = data; !end_of_header(p); p = next_header_line(p)) {
-		if (skip_prefix(p, "tagger ", &p)) {
+		if (skip_prefix(p, "tagger ", &p))
 			parse_user(p, &info->tagger, &info->tagger_email, &info->tagger_date, &info->tagger_tz);
-		}
 	}
 
 	while (p && *p == '\n')
diff --git a/source/parsing.h b/source/parsing.h
index d213429..755d646 100644
--- a/source/parsing.h
+++ b/source/parsing.h
@@ -10,8 +10,10 @@
 
 #include "cgit.h"
 
-// Split a request url into the repository, the page and the path it names,
-// storing them in ctx.qry.
+/*
+ * Split a request url into the repository, the page and the path it names,
+ * storing them in ctx.qry.
+ */
 extern void cgit_parse_url(const char *url);
 
 extern struct string_list *cgit_mailmap(void);
diff --git a/source/scan-tree.c b/source/scan-tree.c
index ba2149a..4110511 100644
--- a/source/scan-tree.c
+++ b/source/scan-tree.c
@@ -32,7 +32,7 @@ static int stat_entry(const char *dir, const char *name, struct stat *st)
 	// A missing entry is the ordinary answer for a directory that is not a
 	// repository, so only some other failure is worth reporting.
 	if (err && errno != ENOENT)
-		fprintf(stderr, "[cgit] Error checking path %s: %s (%d)\n", dir, strerror(errno), errno);
+		fprintf(stderr, "[cgit] Unable to stat %s: %s (%d)\n", dir, strerror(errno), errno);
 	strbuf_release(&path);
 	return err;
 }
@@ -48,13 +48,30 @@ static int is_git_dir(const char *path)
 	return 1;
 }
 
-// A filter is a command cgit runs, and a repository's own files belong to
-// whoever can push to it, so their filter keys wait on trust-scan-filters.
-static int trusted_key(const char *name)
+/*
+ * A repository's own files belong to whoever can push to it, so the keys
+ * that run a command, put raw markup on the page, read a file off the disk
+ * or place a link wait on trust-scan-config. A readme naming a git object,
+ * the form with a colon, only reads from the repository and passes.
+ */
+static int trusted_key(const char *name, const char *value)
 {
-	if (!ends_with(name, "-filter") || ctx.cfg.trust_scan_filters)
+	int untrusted;
+
+	if (ctx.cfg.trust_scan_config)
 		return 1;
-	fprintf(stderr, "[cgit] Ignoring %s in %s: trust-scan-filters is not set\n", name,
+	untrusted =
+		ends_with(name, "-filter")							||
+		!strcmp(name, "head-content")						||
+		!strcmp(name, "module-link")						||
+		starts_with(name, "module-link.")					||
+		!strcmp(name, "logo")								||
+		!strcmp(name, "logo-link")							||
+		!strcmp(name, "clone-url")							||
+		(!strcmp(name, "readme") && !strchr(value, ':'));
+	if (!untrusted)
+		return 1;
+	fprintf(stderr, "[cgit] Ignoring %s in %s: trust-scan-config is not set\n", name,
 		current_repo->path);
 	return 0;
 }
@@ -64,13 +81,16 @@ static int apply_gitconfig(const char *key, const char *value,
 {
 	const char *name;
 
+	// A key with no value is legal git config and nothing here can use it.
+	if (!value)
+		return 0;
 	if (!strcmp(key, "gitweb.owner"))
 		cgit_repo_config(current_repo, "owner", value);
 	else if (!strcmp(key, "gitweb.description"))
 		cgit_repo_config(current_repo, "desc", value);
 	else if (!strcmp(key, "gitweb.category"))
 		cgit_repo_config(current_repo, "section", value);
-	else if (skip_prefix(key, "cgit.", &name) && trusted_key(name))
+	else if (skip_prefix(key, "cgit.", &name) && trusted_key(name, value))
 		cgit_repo_config(current_repo, name, value);
 
 	return 0;
@@ -78,7 +98,7 @@ static int apply_gitconfig(const char *key, const char *value,
 
 static void apply_cgitrc(const char *name, const char *value)
 {
-	if (trusted_key(name))
+	if (trusted_key(name, value))
 		cgit_repo_config(current_repo, name, value);
 }
 
@@ -135,7 +155,7 @@ static void add_repo(const char *base, struct strbuf *path)
 	size_t desc_size;
 
 	if (stat(path->buf, &st)) {
-		fprintf(stderr, "[cgit] Error accessing %s: %s (%d)\n", path->buf, strerror(errno), errno);
+		fprintf(stderr, "[cgit] Unable to access %s: %s (%d)\n", path->buf, strerror(errno), errno);
 		return;
 	}
 
@@ -143,8 +163,10 @@ static void add_repo(const char *base, struct strbuf *path)
 	pathlen = path->len;
 
 	if (ctx.cfg.strict_export) {
+		struct stat export_st;
+
 		strbuf_addstr(path, ctx.cfg.strict_export);
-		if (stat(path->buf, &st))
+		if (stat(path->buf, &export_st))
 			return;
 		strbuf_setlen(path, pathlen);
 	}
@@ -165,11 +187,31 @@ static void add_repo(const char *base, struct strbuf *path)
 		strbuf_setlen(&relpath, relpath.len - 1);
 	if (relpath.len >= 5 && !strcmp(relpath.buf + relpath.len - 5, "/.git"))
 		strbuf_setlen(&relpath, relpath.len - 5);
+	else if (!strcmp(relpath.buf, ".git"))
+		strbuf_setlen(&relpath, 0);
+	// The scan root may itself be the repository, leaving nothing after
+	// the base, so that one is named after its directory.
+	if (!relpath.len) {
+		const char *end = path->buf + pathlen - 1, *start;
+
+		if (end - path->buf >= 5 && !strncmp(end - 5, "/.git", 5))
+			end -= 5;
+		start = end;
+		while (start > path->buf && start[-1] != '/')
+			start--;
+		strbuf_add(&relpath, start, end - start);
+	}
 
 	current_repo = cgit_add_repo(relpath.buf);
 	if (ctx.cfg.enable_git_config) {
+		// A pusher wrote this file, so a broken one is skipped with a
+		// warning rather than allowed to end the request.
+		struct config_options opts = { .error_action = CONFIG_ERROR_SILENT };
+
 		strbuf_addstr(path, "config");
-		git_config_from_file(apply_gitconfig, path->buf, NULL);
+		if (git_config_from_file_with_options(apply_gitconfig, path->buf, NULL,
+				CONFIG_SCOPE_UNKNOWN, &opts))
+			fprintf(stderr, "[cgit] Ignoring unreadable config in %s\n", path->buf);
 		strbuf_setlen(path, pathlen);
 	}
 
@@ -181,16 +223,15 @@ static void add_repo(const char *base, struct strbuf *path)
 	}
 	current_repo->path = cgit_strdup_first_line(path->buf);
 	while (!current_repo->owner) {
-		if ((pwd = getpwuid(st.st_uid)) == NULL) {
-			fprintf(stderr, "[cgit] Error reading owner-info for %s: %s (%d)\n",
+		if (!(pwd = getpwuid(st.st_uid))) {
+			fprintf(stderr, "[cgit] Unable to read the owner of %s: %s (%d)\n",
 				path->buf, strerror(errno), errno);
 			break;
 		}
 		// A gecos field puts the owner's name in front of a comma
 		// separated list of office and phone details.
-		if (pwd->pw_gecos)
-			if ((comma = strchr(pwd->pw_gecos, ',')))
-				*comma = '\0';
+		if (pwd->pw_gecos && (comma = strchr(pwd->pw_gecos, ',')))
+			*comma = '\0';
 		current_repo->owner = cgit_strdup_first_line(pwd->pw_gecos ? pwd->pw_gecos : pwd->pw_name);
 	}
 
@@ -210,7 +251,7 @@ static void add_repo(const char *base, struct strbuf *path)
 
 	strbuf_addstr(path, "cgitrc");
 	if (!stat(path->buf, &st))
-		config_file_parse(path->buf, &apply_cgitrc);
+		config_file_parse(path->buf, apply_cgitrc);
 
 	strbuf_release(&relpath);
 }
@@ -226,16 +267,45 @@ static int should_scan(const struct dirent *ent)
 	return ctx.cfg.scan_hidden_path;
 }
 
+// Symlinks are followed so that a link into the scan path counts, which means
+// a link back at an ancestor would recurse until the path ran out of room.
+// Each directory is entered once.
+static struct {
+	dev_t dev;
+	ino_t ino;
+} *visited;
+static size_t visited_nr, visited_alloc;
+
+static int already_visited(const char *path)
+{
+	struct stat st;
+	size_t i;
+
+	if (stat(path, &st))
+		return 0;
+	for (i = 0; i < visited_nr; i++)
+		if (visited[i].dev == st.st_dev && visited[i].ino == st.st_ino)
+			return 1;
+	ALLOC_GROW(visited, visited_nr + 1, visited_alloc);
+	visited[visited_nr].dev = st.st_dev;
+	visited[visited_nr].ino = st.st_ino;
+	visited_nr++;
+	return 0;
+}
+
 static void scan_path(const char *base, const char *path)
 {
-	DIR *dir = opendir(path);
+	DIR *dir;
 	struct dirent *ent;
 	struct strbuf pathbuf = STRBUF_INIT;
 	size_t pathlen = strlen(path);
 	struct stat st;
 
+	if (already_visited(path))
+		return;
+	dir = opendir(path);
 	if (!dir) {
-		fprintf(stderr, "[cgit] Error opening directory %s: %s (%d)\n", path, strerror(errno), errno);
+		fprintf(stderr, "[cgit] Unable to open %s: %s (%d)\n", path, strerror(errno), errno);
 		return;
 	}
 
@@ -252,13 +322,14 @@ static void scan_path(const char *base, const char *path)
 	// Take in the '/' that "/.git" left in the buffer, since the loop below
 	// truncates to this length and then appends an entry name straight on.
 	pathlen++;
-	while ((ent = readdir(dir)) != NULL) {
+	while ((ent = readdir(dir))) {
 		if (!should_scan(ent))
 			continue;
 		strbuf_setlen(&pathbuf, pathlen);
 		strbuf_addstr(&pathbuf, ent->d_name);
 		if (stat(pathbuf.buf, &st)) {
-			fprintf(stderr, "[cgit] Error checking path %s: %s (%d)\n", pathbuf.buf, strerror(errno), errno);
+			fprintf(stderr, "[cgit] Unable to stat %s: %s (%d)\n", pathbuf.buf, strerror(errno),
+				errno);
 			continue;
 		}
 		if (S_ISDIR(st.st_mode))
@@ -277,7 +348,7 @@ void scan_projects(const char *path, const char *projectsfile)
 
 	projects = fopen(projectsfile, "r");
 	if (!projects) {
-		fprintf(stderr, "[cgit] Error opening projectsfile %s: %s (%d)\n",
+		fprintf(stderr, "[cgit] Unable to open project list %s: %s (%d)\n",
 			projectsfile, strerror(errno), errno);
 		return;
 	}
@@ -289,7 +360,7 @@ void scan_projects(const char *path, const char *projectsfile)
 		scan_path(path, line.buf);
 	}
 	if ((err = ferror(projects))) {
-		fprintf(stderr, "[cgit] Error reading from projectsfile %s: %s (%d)\n",
+		fprintf(stderr, "[cgit] Unable to read project list %s: %s (%d)\n",
 			projectsfile, strerror(err), err);
 	}
 	fclose(projects);
diff --git a/source/shared.c b/source/shared.c
index 6ac2353..a1d1597 100644
--- a/source/shared.c
+++ b/source/shared.c
@@ -14,8 +14,8 @@
 
 #define MACRO_EXPANSION_BUFSIZE (1024 * 8)
 
-// The number of context lines git itself defaults to.
-#define DEFAULT_DIFF_CONTEXT 3
+// How much of a file is read to find its first line.
+#define MAX_FIRST_LINE_READ (1024 * 64)
 
 typedef struct {
 	const char *name;
@@ -68,9 +68,29 @@ static struct refinfo *make_refinfo(const char *refname, const struct object_id
 	return ref;
 }
 
+/*
+ * Whether a revision from the request may reach git. A hex object id or a name
+ * that could be a ref passes. Git's wider syntax is refused, since :/pattern
+ * and rev^{/pattern} walk the whole history for a match, and a leading dash
+ * reads as an option further down.
+ */
+int cgit_valid_rev(const char *rev)
+{
+	const char *p = rev;
+
+	if (!rev || !*rev || *rev == '-')
+		return 0;
+	while (isxdigit((unsigned char)*p))
+		p++;
+	if (!*p)
+		return 1;
+	return !check_refname_format(rev, REFNAME_ALLOW_ONELEVEL);
+}
+
 static int load_mmfile(mmfile_t *file, const struct object_id *oid)
 {
 	enum object_type type;
+	size_t size;
 
 	// A null oid is the absent side of an add or a delete, and diffs as an
 	// empty file rather than as a failure.
@@ -80,10 +100,11 @@ static int load_mmfile(mmfile_t *file, const struct object_id *oid)
 		return 1;
 	}
 
-	file->ptr = odb_read_object(the_repository->objects, oid, &type, (unsigned long *)&file->size);
-	// odb_read_object leaves size untouched when it fails, so the caller
-	// has to be told rather than handed a buffer with an unset length.
-	return file->ptr != NULL;
+	file->ptr = odb_read_object(the_repository->objects, oid, &type, &size);
+	if (!file->ptr)
+		return 0;
+	file->size = size;
+	return 1;
 }
 
 /*
@@ -106,7 +127,7 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf)
 	int i;
 
 	for (i = 0; i < nbuf; i++) {
-		if (mb[i].ptr[mb[i].size-1] != '\n') {
+		if (mb[i].ptr[mb[i].size - 1] != '\n') {
 			fragment = xrealloc(fragment, fragment_len + mb[i].size);
 			memcpy(fragment + fragment_len, mb[i].ptr, mb[i].size);
 			fragment_len += mb[i].size;
@@ -133,8 +154,10 @@ static int emit_line(void *priv, mmbuffer_t *mb, int nbuf)
 	return 0;
 }
 
-// Takes an unsigned char because a byte over 0x7f is negative where char
-// is signed, which the ctype tests are not defined for.
+/*
+ * Takes an unsigned char because a byte over 0x7f is negative where char is
+ * signed, which the ctype tests are not defined for.
+ */
 static int is_token_char(unsigned char c)
 {
 	return isalnum(c) || c == '_';
@@ -192,14 +215,22 @@ struct cgit_repo *cgit_add_repo(const char *url)
 			cgit_repolist.length = 8;
 		else
 			cgit_repolist.length *= 2;
-		cgit_repolist.repos = xrealloc(cgit_repolist.repos, cgit_repolist.length * sizeof(struct cgit_repo));
+		cgit_repolist.repos =
+			xrealloc(cgit_repolist.repos, cgit_repolist.length * sizeof(struct cgit_repo));
 	}
 
-	repo = &cgit_repolist.repos[cgit_repolist.count-1];
+	repo = &cgit_repolist.repos[cgit_repolist.count - 1];
 	memset(repo, 0, sizeof(struct cgit_repo));
 	repo->url = cgit_trim_end(url, '/');
-	if (repo->url)
+	if (repo->url) {
 		*strchrnul(repo->url, '\n') = '\0';
+	} else {
+		// Nothing can address a repository with no url, and every lookup
+		// compares the url, so it is kept out of the way.
+		fprintf(stderr, "[cgit] Ignoring repository with an empty url\n");
+		repo->url = xstrdup("");
+		repo->ignore = 1;
+	}
 	repo->name = repo->url;
 	repo->path = NULL;
 	repo->desc = cgit_default_repo_desc;
@@ -222,7 +253,6 @@ struct cgit_repo *cgit_add_repo(const char *url)
 	repo->branch_sort = ctx.cfg.branch_sort;
 	repo->commit_sort = ctx.cfg.commit_sort;
 	repo->module_link = ctx.cfg.module_link;
-	repo->readme = ctx.cfg.readme;
 	repo->mtime = -1;
 	repo->about_filter = ctx.cfg.about_filter;
 	repo->commit_filter = ctx.cfg.commit_filter;
@@ -266,7 +296,7 @@ char *cgit_trim_end(const char *str, char c)
 {
 	size_t len;
 
-	if (str == NULL)
+	if (!str)
 		return NULL;
 	len = strlen(str);
 	while (len > 0 && str[len - 1] == c)
@@ -286,7 +316,7 @@ char *cgit_ensure_end(const char *str, char c)
 
 	result = xmalloc(len + 2);
 	memcpy(result, str, len);
-	result[len] = '/';
+	result[len] = c;
 	result[len + 1] = '\0';
 	return result;
 }
@@ -367,9 +397,15 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new
 	// Read the object headers first so an oversized blob is never inflated
 	// into memory just to be diffed. Reporting it as binary suppresses
 	// inlining the same way max-blob-size does in the other views.
-	if (!is_null_oid(old_oid) && odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0)
+	if (
+		!is_null_oid(old_oid) &&
+		odb_read_object_info(the_repository->objects, old_oid, &old_bytes) < 0
+	)
 		return 1;
-	if (!is_null_oid(new_oid) && odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0)
+	if (
+		!is_null_oid(new_oid) &&
+		odb_read_object_info(the_repository->objects, new_oid, &new_bytes) < 0
+	)
 		return 1;
 
 	*old_size = old_bytes;
@@ -389,7 +425,10 @@ int cgit_diff_files(const struct object_id *old_oid, const struct object_id *new
 		return 1;
 	}
 
-	if (buffer_is_binary(old_file.ptr, old_file.size) || buffer_is_binary(new_file.ptr, new_file.size)) {
+	if (
+		buffer_is_binary(old_file.ptr, old_file.size) ||
+		buffer_is_binary(new_file.ptr, new_file.size)
+	) {
 		*binary = 1;
 		release_mmfile(&old_file, old_oid);
 		release_mmfile(&new_file, new_oid);
@@ -431,6 +470,9 @@ void cgit_diff_tree(const struct object_id *old_oid, const struct object_id *new
 		item = xcalloc(1, sizeof(*item));
 		item->match = xstrdup(prefix);
 		item->len = strlen(prefix);
+		// nowildcard_len matching len makes git treat the path as
+		// literal rather than as a glob.
+		item->nowildcard_len = item->len;
 		opt.pathspec.nr = 1;
 		opt.pathspec.items = item;
 	}
@@ -500,7 +542,7 @@ void cgit_prepare_repo_env(struct cgit_repo *repo)
 
 	for (i = 0; i < ARRAY_SIZE(vars); i++)
 		if (vars[i].value && setenv(vars[i].name, vars[i].value, 1))
-			fprintf(stderr, "[cgit] Error setting env %s=%s: %s (%d)\n",
+			fprintf(stderr, "[cgit] Unable to set %s=%s: %s (%d)\n",
 				vars[i].name, vars[i].value, strerror(errno), errno);
 }
 
@@ -508,6 +550,7 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size)
 {
 	int fd, err;
 	ssize_t got;
+	size_t want;
 	struct stat st;
 
 	fd = open(path, O_RDONLY);
@@ -522,10 +565,13 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size)
 		close(fd);
 		return EISDIR;
 	}
-	*buf = xmalloc(st.st_size + 1);
-	got = read_in_full(fd, *buf, st.st_size);
-	err = errno;
+	// Only the first line is wanted, and a description file in a scanned
+	// repository is whatever a pusher wrote, so the read is bounded.
+	want = st.st_size < MAX_FIRST_LINE_READ ? st.st_size : MAX_FIRST_LINE_READ;
+	*buf = xmalloc(want + 1);
+	got = read_in_full(fd, *buf, want);
 	if (got < 0) {
+		err = errno;
 		free(*buf);
 		*buf = NULL;
 		*size = 0;
@@ -536,12 +582,13 @@ int cgit_read_first_line(const char *path, char **buf, size_t *size)
 	(*buf)[*size] = '\0';
 	*strchrnul(*buf, '\n') = '\0';
 	close(fd);
-	return (*size == (size_t)st.st_size ? 0 : err);
+	return 0;
 }
 
 char *cgit_strdup_first_line(const char *text)
 {
 	char *line = xstrdup(text);
+
 	*strchrnul(line, '\n') = '\0';
 	return line;
 }
@@ -561,11 +608,13 @@ char *cgit_expand_macros(const char *text)
 				if (out - start > 0) {
 					*out = '\0';
 					out = expand_macro(start, limit - start) - 1;
+					// Step back so the byte that ended the
+					// token is written again and may open a
+					// new one. A bare dollar expanded nothing
+					// and its follower is already in place.
+					text--;
 				}
 				start = NULL;
-				// Step back so the byte that ended the token
-				// is written again and may open a new one.
-				text--;
 			}
 			out++;
 			text++;
@@ -621,6 +670,10 @@ char *cgit_get_mimetype_for_filename(const char *filename)
 		// extension that maps to it.
 		string_list_split_in_place(&fields, line, " \t\r\n", -1);
 		string_list_remove_empty_items(&fields, 0);
+		if (!fields.nr) {
+			string_list_clear(&fields, 0);
+			continue;
+		}
 		type = fields.items[0].string;
 		for (i = 1; i < fields.nr; i++) {
 			if (!strcasecmp(ext, fields.items[i].string)) {
diff --git a/source/shared.h b/source/shared.h
index b9f6ecf..9e39389 100644
--- a/source/shared.h
+++ b/source/shared.h
@@ -32,13 +32,17 @@ extern int cgit_die_unless_non_negative(int result, const char *msg);
 extern struct cgit_repo *cgit_add_repo(const char *url);
 extern struct cgit_repo *cgit_get_repoinfo(const char *url);
 
-// Export the CGIT_REPO_* variables that filters and hooks read.
+/*
+ * Export the CGIT_REPO_* variables that filters and hooks read.
+ */
 extern void cgit_prepare_repo_env(struct cgit_repo *repo);
 
 extern void cgit_free_reflist_inner(struct reflist *list);
 
-// Matches git's reference iteration callback, and appends each ref it is
-// given to the struct reflist passed as cb_data.
+/*
+ * Matches git's reference iteration callback, and appends each ref it is given
+ * to the struct reflist passed as cb_data.
+ */
 extern int cgit_refs_cb(const struct reference *ref, void *cb_data);
 
 extern void cgit_free_commitinfo(struct commitinfo *info);
@@ -60,11 +64,15 @@ extern void cgit_diff_tree(const struct object_id *old_oid, const struct object_
 	filepair_fn fn, const char *prefix, int ignorews);
 extern void cgit_diff_commit(struct commit *commit, filepair_fn fn, const char *prefix);
 
-// Accept only the date formats cgit documents, leaving mode unchanged for
-// anything else.
+/*
+ * Accept only the date formats cgit documents, leaving mode unchanged for
+ * anything else.
+ */
 extern void cgit_parse_date_format(const char *format, struct date_mode *mode);
 
-// Copy str without its trailing runs of c, returning NULL if nothing is left.
+/*
+ * Copy str without its trailing runs of c, returning NULL if nothing is left.
+ */
 extern char *cgit_trim_end(const char *str, char c);
 extern char *cgit_ensure_end(const char *str, char c);
 extern void strbuf_ensure_end(struct strbuf *sb, char c);
@@ -84,4 +92,10 @@ extern char *cgit_expand_macros(const char *text);
 
 extern char *cgit_get_mimetype_for_filename(const char *filename);
 
+/*
+ * Whether a revision from the request is a hex object id or a possible ref
+ * name, the only forms cgit hands to git.
+ */
+extern int cgit_valid_rev(const char *rev);
+
 #endif // CGIT_SHARED_H