From 72601ca556210e1d06e05a03e11ac87fd4903aee Mon Sep 17 00:00:00 2001 From: Bryce Kwon Date: Mon, 2 Nov 2026 19:06:38 -1000 Subject: Say when logins are unavailable The way back was signed part way through the form, so while the secret's directory did not exist, or a library was missing, cgit's error landed inside the hidden input and the page broke off there with no fields. --- custom/extensions/auth-file.lua | 25 +++++++++++++++++++------ custom/extensions/auth-inline.lua | 25 +++++++++++++++++++------ tests/extensions/harness.lua | 21 +++++++++++++++++++++ tests/extensions/test-auth.lua | 17 ++++++++++++++++- tests/t0505-auth.sh | 5 +++-- 5 files changed, 78 insertions(+), 15 deletions(-) diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua index ba05875..3814a1f 100644 --- a/custom/extensions/auth-file.lua +++ b/custom/extensions/auth-file.lua @@ -91,10 +91,11 @@ local groups_filename = "/etc/cgit-auth/groups" -- protected ones apart. local repos_filename = "/etc/cgit-auth/repos" --- Where the cookie-signing secret is stored, created on first use. It must be --- persistent and writable by cgit, and it is worth keeping outside the cache --- root, because pruning the cache would delete a secret kept inside it and --- invalidate every live session. This file should not be world-readable. +-- Where the cookie-signing secret is stored, created on first use in a +-- directory that has to exist already. It must be persistent and writable by +-- cgit, and it is worth keeping outside the cache root, because pruning the +-- cache would delete a secret kept inside it and invalidate every live +-- session. This file should not be world-readable. local secret_filename = "/var/lib/cgit/auth-secret" -- How long a login stays valid, in seconds. The default is one week. @@ -295,7 +296,9 @@ function get_secret() local temporary_file = io.open(temporary_filename, "w") if temporary_file == nil then sysstat.umask(old_umask) - error("cgit auth: cannot create secret file " .. secret_filename) + local directory = secret_filename:match("^(.*)/") or "." + error("cgit auth: cannot create secret file " .. secret_filename .. + ", as " .. directory .. " is missing or not writable by cgit") end local wrote = temporary_file:write(tohex(rand.bytes(32))) local closed = temporary_file:close() @@ -520,6 +523,16 @@ function body() end html("

Authentication Required

") + -- The token is signed before any of the form is written, since an error + -- raised part way through would land inside its markup. A missing + -- library or a secret that cannot be made leaves a page saying logins + -- are unavailable instead, with the reason in the error log. + local signed, token = pcall(secure_value, "redirect", target, 0) + if not signed then + io.stderr:write("[cgit] " .. tostring(token):match("[^\n]*") .. "\n") + html("") + return 0 + end if failed then html("") end @@ -527,7 +540,7 @@ function body() html_attr(cgit["login"]) html("'>") html("") html("") html("
") diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua index 971d662..e7f12cd 100644 --- a/custom/extensions/auth-inline.lua +++ b/custom/extensions/auth-inline.lua @@ -92,10 +92,11 @@ local users = { -- bob = "$6$rounds=300000$REPLACE_THIS_SALT$REPLACE_THIS_HASH", } --- Where the cookie-signing secret is stored, created on first use. It must be --- persistent and writable by cgit, and it is worth keeping outside the cache --- root, because pruning the cache would delete a secret kept inside it and --- invalidate every live session. This file should not be world-readable. +-- Where the cookie-signing secret is stored, created on first use in a +-- directory that has to exist already. It must be persistent and writable by +-- cgit, and it is worth keeping outside the cache root, because pruning the +-- cache would delete a secret kept inside it and invalidate every live +-- session. This file should not be world-readable. local secret_filename = "/var/lib/cgit/auth-secret" -- How long a login stays valid, in seconds. The default is one week. @@ -258,7 +259,9 @@ function get_secret() local temporary_file = io.open(temporary_filename, "w") if temporary_file == nil then sysstat.umask(old_umask) - error("cgit auth: cannot create secret file " .. secret_filename) + local directory = secret_filename:match("^(.*)/") or "." + error("cgit auth: cannot create secret file " .. secret_filename .. + ", as " .. directory .. " is missing or not writable by cgit") end local wrote = temporary_file:write(tohex(rand.bytes(32))) local closed = temporary_file:close() @@ -483,6 +486,16 @@ function body() end html("

Authentication Required

") + -- The token is signed before any of the form is written, since an error + -- raised part way through would land inside its markup. A missing + -- library or a secret that cannot be made leaves a page saying logins + -- are unavailable instead, with the reason in the error log. + local signed, token = pcall(secure_value, "redirect", target, 0) + if not signed then + io.stderr:write("[cgit] " .. tostring(token):match("[^\n]*") .. "\n") + html("") + return 0 + end if failed then html("") end @@ -490,7 +503,7 @@ function body() html_attr(cgit["login"]) html("'>") html("") html("") html("
") diff --git a/tests/extensions/harness.lua b/tests/extensions/harness.lua index 2fe722f..8c1ba6c 100644 --- a/tests/extensions/harness.lua +++ b/tests/extensions/harness.lua @@ -54,14 +54,30 @@ html_url_path = sink(function(s) end)) end) +-- What a script writes to standard error goes to the web server's error log +-- under cgit, so it is kept here for a check to read instead. +local logged = {} + +io.stderr = { + write = function(self, ...) + logged[#logged + 1] = table.concat({ ... }) + return self + end, +} + function harness.reset() pieces = {} + logged = {} end function harness.output() return table.concat(pieces) end +function harness.error_log() + return table.concat(logged) +end + function harness.load(script) dofile(script) end @@ -182,6 +198,11 @@ function harness.stub_auth_modules() preload("posix.unistd", { crypt = fake_crypt, }) + preload("posix.sys.stat", { + umask = function() + return 18 + end, + }) preload("openssl.rand", { bytes = function(count) local out = {} diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua index 974544d..3a4fabf 100644 --- a/tests/extensions/test-auth.lua +++ b/tests/extensions/test-auth.lua @@ -302,6 +302,18 @@ h.contains("a failed login says so", out, "") h.equals("and leaves the flag out of the way back", way_back_in(out), "/repo/log/?q=x") +-- The real get_secret, pointed at a directory nobody made, cannot sign the +-- way back, so the page says so whole rather than breaking off in the form. +h.load_with(script, { { "local secret_filename = \"/var/lib/cgit/auth-secret\"", + "local secret_filename = \"no-such-directory/auth-secret\"" } }) +out = run_action("body", {}) +h.contains("a secret that cannot be made leaves a page saying so", out, + "") +h.excludes("with no form to fill in", out, "tmp && + cgit_rc guardedrc "url=authy/log/" >tmp 2>err && ! grep "guarded commit" tmp && - grep "Unable to complete the request" tmp + grep "

Logins are unavailable" tmp && + grep "^\[cgit\] " err ' # A filter of the test's own turns every visitor away, which needs no Lua. -- cgit 2.12.1