WIP: Add require signed commits feature #8584

lunny · 2019-10-19T02:23:40Z

Fix #4249, This PR is not work yet because on prereceive hook, you should not get the commit information from the repository.

zeripath

Some spelling mistakes and a hint as to how to get the git commands to work

zeripath · 2019-10-19T06:23:56Z

routers/private/hook.go

+				return
+			}
+
+			stdout, err := git.NewCommand("rev-list", oldCommitID+"..."+newCommitID).RunInDirBytes(repo.RepoPath())


In pre-receive the new commits and objects are not placed directly into the object tree (depending on your git version of course). They are placed in to Quarantine and/or various other places.

You need to set the environment for any git commands to tell it to look in those directories for objects too.

gitea/routers/private/hook.go

Lines 67 to 82 in 0a004a6

env := os.Environ()

if gitAlternativeObjectDirectories != "" {

env = append(env,

private.GitAlternativeObjectDirectories+"="+gitAlternativeObjectDirectories)

}

if gitObjectDirectory != "" {

env = append(env,

private.GitObjectDirectory+"="+gitObjectDirectory)

}

if gitQuarantinePath != "" {

env = append(env,

private.GitQuarantinePath+"="+gitQuarantinePath)

}

output, err := git.NewCommand("rev-list", "--max-count=1", oldCommitID, "^"+newCommitID).RunInDirWithEnv(repo.RepoPath(), env)

if err != nil {

I wouldn't use OpenRepository either just use the git repo commands directly. If you want to use gogit commands I'm fairly certain you can set its environs similarly.

zeripath · 2019-10-19T06:24:36Z

options/locale/locale_en-US.ini

@@ -1336,6 +1336,8 @@ settings.protect_merge_whitelist_teams = Whitelisted teams for merging:
 settings.protect_check_status_contexts = Enable Status Check
 settings.protect_check_status_contexts_desc = Require status checks to pass before merging Choose which status checks must pass before branches can be merged into a branch that matches this rule. When enabled, commits must first be pushed to another branch, then merged or pushed directly to a branch that matches this rule after status checks have passed. If no contexts are selected, the last commit must be successful regardless of context.
 settings.protect_check_status_contexts_list = Status checks found in the last week for this repository
+settings.requrie_signed_commits = Require signed commits


Suggested change

settings.requrie_signed_commits = Require signed commits

settings.require_signed_commits = Require signed commits

zeripath · 2019-10-19T06:24:51Z

options/locale/locale_en-US.ini

@@ -1336,6 +1336,8 @@ settings.protect_merge_whitelist_teams = Whitelisted teams for merging:
 settings.protect_check_status_contexts = Enable Status Check
 settings.protect_check_status_contexts_desc = Require status checks to pass before merging Choose which status checks must pass before branches can be merged into a branch that matches this rule. When enabled, commits must first be pushed to another branch, then merged or pushed directly to a branch that matches this rule after status checks have passed. If no contexts are selected, the last commit must be successful regardless of context.
 settings.protect_check_status_contexts_list = Status checks found in the last week for this repository
+settings.requrie_signed_commits = Require signed commits
+settings.requrie_signed_commits_desc = Commits pushed to this branch must have verified signatures.


Suggested change

settings.requrie_signed_commits_desc = Commits pushed to this branch must have verified signatures.

settings.require_signed_commits_desc = Commits pushed to this branch must have verified signatures.

zeripath · 2019-10-19T06:25:39Z

templates/repo/settings/protected_branch.tmpl

+					<div class="field">
+						<div class="ui checkbox">
+							<input class="require-signedcommits" name="require_signed_commits" type="checkbox" {{if .Branch.RequireSignedCommits}}checked{{end}}>
+							<label>{{.i18n.Tr "repo.settings.requrie_signed_commits"}}</label>


Suggested change

<label>{{.i18n.Tr "repo.settings.requrie_signed_commits"}}</label>

<label>{{.i18n.Tr "repo.settings.require_signed_commits"}}</label>

zeripath · 2019-10-19T06:25:52Z

templates/repo/settings/protected_branch.tmpl

+						<div class="ui checkbox">
+							<input class="require-signedcommits" name="require_signed_commits" type="checkbox" {{if .Branch.RequireSignedCommits}}checked{{end}}>
+							<label>{{.i18n.Tr "repo.settings.requrie_signed_commits"}}</label>
+							<p class="help">{{.i18n.Tr "repo.settings.requrie_signed_commits_desc"}}</p>


Suggested change

<p class="help">{{.i18n.Tr "repo.settings.requrie_signed_commits_desc"}}</p>

<p class="help">{{.i18n.Tr "repo.settings.require_signed_commits_desc"}}</p>

zeripath · 2019-10-19T06:42:42Z

Of course, this doesn't solve the issue of merge and web commits - we still need some way of indicating whether such a commit would be signed and if not possibly disable their merge/submit buttons.

As an aside what happens when you try to use the web editor to commit to a protected branch? Presumably it would fail - but do we nicely disable the submit button?

guillep2k · 2019-10-19T12:14:13Z

Of course, this doesn't solve the issue of merge and web commits - we still need some way of indicating whether such a commit would be signed and if not possibly disable their merge/submit buttons.

As an aside what happens when you try to use the web editor to commit to a protected branch? Presumably it would fail - but do we nicely disable the submit button?

If I understand your question correctly, yes: the only option available when the branch is protected is "create new branch".

zeripath · 2019-10-19T14:51:42Z

@guillep2k excellent. I'm sure I've looked at this code before but just forgot.

Ok. So for CRUD we'd need to adjust whatever code does that check to add a check using:

gitea/models/repo_sign.go

Line 169 in 5a62ae5

    
           func (repo *Repository) SignCRUDAction(u *User, tmpBasePath, parentCommit string) (bool, string) {

tmpBasePath just has to be either the repo or a temporary copy. You don't have to have a temporary copy for this one. If that test fails then we would have to determine why or just say that the branch requires signed commits. It could be that that method could be changed to return the proximal failure reason.

For merges it could just be on attempt to merge that it fails immediately, we do an individual test on display of the UI if the merge button would be displayed or we could cache the results of the individual tests and recombine them on display at the UI depending on the users 2fa status if that matters.

stale · 2019-12-19T01:07:09Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

lunny · 2020-01-12T01:02:51Z

replaced by #9708

add require signed commits feature

b39e13d

zeripath reviewed Oct 19, 2019

View reviewed changes

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 19, 2019

zeripath added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Oct 19, 2019

zeripath mentioned this pull request Nov 10, 2019

Committer verification #2770

Closed

stale bot added the issue/stale label Dec 19, 2019

lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Dec 19, 2019

stale bot removed the issue/stale label Dec 19, 2019

6543 mentioned this pull request Jan 11, 2020

Add require signed commit for protected branch #9708

Merged

lunny closed this Jan 12, 2020

go-gitea locked and limited conversation to collaborators Nov 24, 2020

lunny deleted the lunny/require_signed_commits branch August 24, 2023 11:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Add require signed commits feature #8584

WIP: Add require signed commits feature #8584

lunny commented Oct 19, 2019

zeripath left a comment

zeripath Oct 19, 2019 •

edited

Loading

zeripath Oct 19, 2019

zeripath Oct 19, 2019

zeripath Oct 19, 2019

zeripath Oct 19, 2019

zeripath commented Oct 19, 2019

guillep2k commented Oct 19, 2019

zeripath commented Oct 19, 2019 •

edited

Loading

stale bot commented Dec 19, 2019

lunny commented Jan 12, 2020

	env := os.Environ()
	if gitAlternativeObjectDirectories != "" {
	env = append(env,
	private.GitAlternativeObjectDirectories+"="+gitAlternativeObjectDirectories)
	}
	if gitObjectDirectory != "" {
	env = append(env,
	private.GitObjectDirectory+"="+gitObjectDirectory)
	}
	if gitQuarantinePath != "" {
	env = append(env,
	private.GitQuarantinePath+"="+gitQuarantinePath)
	}

	output, err := git.NewCommand("rev-list", "--max-count=1", oldCommitID, "^"+newCommitID).RunInDirWithEnv(repo.RepoPath(), env)
	if err != nil {

	settings.requrie_signed_commits = Require signed commits
	settings.require_signed_commits = Require signed commits

	settings.requrie_signed_commits_desc = Commits pushed to this branch must have verified signatures.
	settings.require_signed_commits_desc = Commits pushed to this branch must have verified signatures.

	<label>{{.i18n.Tr "repo.settings.requrie_signed_commits"}}</label>
	<label>{{.i18n.Tr "repo.settings.require_signed_commits"}}</label>

	<p class="help">{{.i18n.Tr "repo.settings.requrie_signed_commits_desc"}}</p>
	<p class="help">{{.i18n.Tr "repo.settings.require_signed_commits_desc"}}</p>

WIP: Add require signed commits feature #8584

WIP: Add require signed commits feature #8584

Conversation

lunny commented Oct 19, 2019

zeripath left a comment

Choose a reason for hiding this comment

zeripath Oct 19, 2019 • edited Loading

Choose a reason for hiding this comment

zeripath Oct 19, 2019

Choose a reason for hiding this comment

zeripath Oct 19, 2019

Choose a reason for hiding this comment

zeripath Oct 19, 2019

Choose a reason for hiding this comment

zeripath Oct 19, 2019

Choose a reason for hiding this comment

zeripath commented Oct 19, 2019

guillep2k commented Oct 19, 2019

zeripath commented Oct 19, 2019 • edited Loading

stale bot commented Dec 19, 2019

lunny commented Jan 12, 2020

zeripath Oct 19, 2019 •

edited

Loading

zeripath commented Oct 19, 2019 •

edited

Loading