Skip to content

maintenance: configure credentials to be silent #1798

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Documentation/config/credential.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ credential.helper::
Note that multiple helpers may be defined. See linkgit:gitcredentials[7]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> @@ -501,8 +525,8 @@ void credential_fill(struct credential *c, int all_capabilities)
>  			    c->helpers.items[i].string);
>  	}
>  
> -	credential_getpass(c);
> -	if (!c->username && !c->password && !c->credential)
> +	if (credential_getpass(c) ||
> +	    (!c->username && !c->password && !c->credential))
>  		die("unable to get password from user");
>  }

This is a fallback mode after credential helpers have failed to fill
and return.  Unless these helpers pay attention to the "interactive"
configuration, they may still get stuck.  So it would be #leftoverbits
to update each credential helpers to do the right thing.

The sample credential-store backend does not have to be updated I
guess ;-)

> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index 7b5ab0eae16..ceb3336a5c4 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -186,6 +186,28 @@ test_expect_success 'clone from password-protected repository' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'credential.interactive=false skips askpass' '
> +	set_askpass bogus nonsense &&
> +	(
> +		GIT_TRACE2_EVENT="$(pwd)/interactive-true" &&
> +		export GIT_TRACE2_EVENT &&
> +		test_must_fail git clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-true-dir &&
> +		test_region credential interactive interactive-true &&
> +
> +		GIT_TRACE2_EVENT="$(pwd)/interactive-false" &&
> +		export GIT_TRACE2_EVENT &&
> +		test_must_fail git -c credential.interactive=false \
> +			clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-false-dir &&
> +		test_region ! credential interactive interactive-false &&
> +
> +		GIT_TRACE2_EVENT="$(pwd)/interactive-never" &&
> +		export GIT_TRACE2_EVENT &&
> +		test_must_fail git -c credential.interactive=never \
> +			clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-never-dir &&
> +		test_region ! credential interactive interactive-never
> +	)
> +'
> +
>  test_expect_success 'clone from auth-only-for-push repository' '
>  	echo two >expect &&
>  	set_askpass wrong &&

for details and examples.

credential.interactive::
By default, Git and any configured credential helpers will ask for
user input when new credentials are required. Many of these helpers
will succeed based on stored credentials if those credentials are
still valid. To avoid the possibility of user interactivity from
Git, set `credential.interactive=false`. Some credential helpers
respect this option as well.

credential.useHttpPath::
When acquiring credentials, consider the "path" component of an http
or https URL to be important. Defaults to false. See
Expand Down
53 changes: 46 additions & 7 deletions builtin/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1766,6 +1766,42 @@ static const char *get_frequency(enum schedule_priority schedule)
}
}

static const char *extraconfig[] = {
"credential.interactive=false",
"core.askPass=true", /* 'true' returns success, but no output. */
NULL
};

static const char *get_extra_config_parameters(void) {
static const char *result = NULL;
struct strbuf builder = STRBUF_INIT;

if (result)
return result;

for (const char **s = extraconfig; s && *s; s++)
strbuf_addf(&builder, "-c %s ", *s);

result = strbuf_detach(&builder, NULL);
return result;
}

static const char *get_extra_launchctl_strings(void) {
static const char *result = NULL;
struct strbuf builder = STRBUF_INIT;

if (result)
return result;

for (const char **s = extraconfig; s && *s; s++) {
strbuf_addstr(&builder, "<string>-c</string>\n");
strbuf_addf(&builder, "<string>%s</string>\n", *s);
}

result = strbuf_detach(&builder, NULL);
return result;
}

/*
* get_schedule_cmd` reads the GIT_TEST_MAINT_SCHEDULER environment variable
* to mock the schedulers that `git maintenance start` rely on.
Expand Down Expand Up @@ -1972,6 +2008,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
"<array>\n"
"<string>%s/git</string>\n"
"<string>--exec-path=%s</string>\n"
"%s" /* For extra config parameters. */
"<string>for-each-repo</string>\n"
"<string>--keep-going</string>\n"
"<string>--config=maintenance.repo</string>\n"
Expand All @@ -1981,7 +2018,8 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
"</array>\n"
"<key>StartCalendarInterval</key>\n"
"<array>\n";
strbuf_addf(&plist, preamble, name, exec_path, exec_path, frequency);
strbuf_addf(&plist, preamble, name, exec_path, exec_path,
get_extra_launchctl_strings(), frequency);

switch (schedule) {
case SCHEDULE_HOURLY:
Expand Down Expand Up @@ -2216,11 +2254,12 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
"<Actions Context=\"Author\">\n"
"<Exec>\n"
"<Command>\"%s\\headless-git.exe\"</Command>\n"
"<Arguments>--exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s</Arguments>\n"
"<Arguments>--exec-path=\"%s\" %s for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s</Arguments>\n"
"</Exec>\n"
"</Actions>\n"
"</Task>\n";
fprintf(tfile->fp, xml, exec_path, exec_path, frequency);
fprintf(tfile->fp, xml, exec_path, exec_path,
get_extra_config_parameters(), frequency);
strvec_split(&child.args, cmd);
strvec_pushl(&child.args, "/create", "/tn", name, "/f", "/xml",
get_tempfile_path(tfile), NULL);
Expand Down Expand Up @@ -2361,8 +2400,8 @@ static int crontab_update_schedule(int run_maintenance, int fd)
"# replaced in the future by a Git command.\n\n");

strbuf_addf(&line_format,
"%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\n",
exec_path, exec_path);
"%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" %s for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\n",
exec_path, exec_path, get_extra_config_parameters());
fprintf(cron_in, line_format.buf, minute, "1-23", "*", "hourly");
fprintf(cron_in, line_format.buf, minute, "0", "1-6", "daily");
fprintf(cron_in, line_format.buf, minute, "0", "0", "weekly");
Expand Down Expand Up @@ -2562,7 +2601,7 @@ static int systemd_timer_write_service_template(const char *exec_path)
"\n"
"[Service]\n"
"Type=oneshot\n"
"ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\n"
"ExecStart=\"%s/git\" --exec-path=\"%s\" %s for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\n"
"LockPersonality=yes\n"
"MemoryDenyWriteExecute=yes\n"
"NoNewPrivileges=yes\n"
Expand All @@ -2572,7 +2611,7 @@ static int systemd_timer_write_service_template(const char *exec_path)
"RestrictSUIDSGID=yes\n"
"SystemCallArchitectures=native\n"
"SystemCallFilter=@system-service\n";
if (fprintf(file, unit, exec_path, exec_path) < 0) {
if (fprintf(file, unit, exec_path, exec_path, get_extra_config_parameters()) < 0) {
error(_("failed to write to '%s'"), filename);
fclose(file);
goto error;
Expand Down
30 changes: 27 additions & 3 deletions credential.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include "strbuf.h"
#include "urlmatch.h"
#include "git-compat-util.h"
#include "trace2.h"
#include "repository.h"

void credential_init(struct credential *c)
{
Expand Down Expand Up @@ -251,14 +253,36 @@ static char *credential_ask_one(const char *what, struct credential *c,
return xstrdup(r);
}

static void credential_getpass(struct credential *c)
static int credential_getpass(struct credential *c)
{
int interactive;
char *value;
if (!git_config_get_maybe_bool("credential.interactive", &interactive) &&
!interactive) {
trace2_data_intmax("credential", the_repository,
"interactive/skipped", 1);
return -1;
}
if (!git_config_get_string("credential.interactive", &value)) {
int same = !strcmp(value, "never");
free(value);
if (same) {
trace2_data_intmax("credential", the_repository,
"interactive/skipped", 1);
return -1;
}
}

trace2_region_enter("credential", "interactive", the_repository);
if (!c->username)
c->username = credential_ask_one("Username", c,
PROMPT_ASKPASS|PROMPT_ECHO);
if (!c->password)
c->password = credential_ask_one("Password", c,
PROMPT_ASKPASS);
trace2_region_leave("credential", "interactive", the_repository);

return 0;
}

int credential_has_capability(const struct credential_capability *capa,
Expand Down Expand Up @@ -501,8 +525,8 @@ void credential_fill(struct credential *c, int all_capabilities)
c->helpers.items[i].string);
}

credential_getpass(c);
if (!c->username && !c->password && !c->credential)
if (credential_getpass(c) ||
(!c->username && !c->password && !c->credential))
die("unable to get password from user");
}

Expand Down
3 changes: 3 additions & 0 deletions scalar.c
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,9 @@ static int cmd_reconfigure(int argc, const char **argv)

the_repository = old_repo;

if (toggle_maintenance(1) >= 0)
succeeded = 1;

loop_end:
if (!succeeded) {
res = -1;
Expand Down
22 changes: 22 additions & 0 deletions t/t5551-http-fetch-smart.sh
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,28 @@ test_expect_success 'clone from password-protected repository' '
test_cmp expect actual
'

test_expect_success 'credential.interactive=false skips askpass' '
set_askpass bogus nonsense &&
(
GIT_TRACE2_EVENT="$(pwd)/interactive-true" &&
export GIT_TRACE2_EVENT &&
test_must_fail git clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-true-dir &&
test_region credential interactive interactive-true &&

GIT_TRACE2_EVENT="$(pwd)/interactive-false" &&
export GIT_TRACE2_EVENT &&
test_must_fail git -c credential.interactive=false \
clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-false-dir &&
test_region ! credential interactive interactive-false &&

GIT_TRACE2_EVENT="$(pwd)/interactive-never" &&
export GIT_TRACE2_EVENT &&
test_must_fail git -c credential.interactive=never \
clone --bare "$HTTPD_URL/auth/smart/repo.git" interactive-never-dir &&
test_region ! credential interactive interactive-never
)
'

test_expect_success 'clone from auth-only-for-push repository' '
echo two >expect &&
set_askpass wrong &&
Expand Down
3 changes: 3 additions & 0 deletions t/t7900-maintenance.sh
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,9 @@ test_expect_success 'start and stop Linux/systemd maintenance' '
test_systemd_analyze_verify "systemd/user/git-maintenance@daily.service" &&
test_systemd_analyze_verify "systemd/user/git-maintenance@weekly.service" &&

grep "core.askPass=true" "systemd/user/git-maintenance@.service" &&
grep "credential.interactive=false" "systemd/user/git-maintenance@.service" &&

printf -- "--user enable --now git-maintenance@%s.timer\n" hourly daily weekly >expect &&
test_cmp expect args &&

Expand Down
7 changes: 5 additions & 2 deletions t/t9210-scalar.sh
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,11 @@ test_expect_success 'scalar reconfigure' '
scalar reconfigure one &&
test true = "$(git -C one/src config core.preloadIndex)" &&
git -C one/src config core.preloadIndex false &&
scalar reconfigure -a &&
test true = "$(git -C one/src config core.preloadIndex)"
rm one/src/cron.txt &&
GIT_TRACE2_EVENT="$(pwd)/reconfigure" scalar reconfigure -a &&
test_path_is_file one/src/cron.txt &&
test true = "$(git -C one/src config core.preloadIndex)" &&
test_subcommand git maintenance start <reconfigure
'

test_expect_success 'scalar reconfigure --all with includeIf.onbranch' '
Expand Down
Loading