feat: Improve branch/tag protection rule to disallow specific file extensions (OD-2588)

This commit is contained in:
Robin Shen 2025-10-31 10:36:21 +08:00
parent 5f5c114cdd
commit 7400902735
13 changed files with 173 additions and 22 deletions

View File

@ -8349,4 +8349,21 @@ public class DataMigrator {
} }
} }
} }
private void migrate213(File dataDir, Stack<Integer> versions) {
for (File file : dataDir.listFiles()) {
if (file.getName().startsWith("Projects.xml")) {
VersionedXmlDoc dom = VersionedXmlDoc.fromFile(file);
for (Element projectElement : dom.getRootElement().elements()) {
for (Element branchProtectionElement : projectElement.element("branchProtections").elements()) {
branchProtectionElement.addElement("disallowedFileTypes");
}
for (Element tagProtectionElement : projectElement.element("tagProtections").elements()) {
tagProtectionElement.addElement("disallowedFileTypes");
}
}
dom.writeToFile(file, false);
}
}
}
} }

View File

@ -6,6 +6,7 @@ import java.util.HashSet;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.util.QuotedString; import org.eclipse.jgit.util.QuotedString;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@ -43,7 +44,10 @@ public class ListChangedFilesCommand {
Commandline git = newGit().workingDir(workingDir); Commandline git = newGit().workingDir(workingDir);
git.environments().putAll(envs); git.environments().putAll(envs);
git.addArgs("diff", "--name-only", "--no-renames", fromRev + ".." + toRev); if (fromRev.equals(ObjectId.zeroId().name()))
git.addArgs("ls-tree", "--name-only", toRev);
else
git.addArgs("diff", "--name-only", "--no-renames", fromRev + ".." + toRev);
git.execute(new LineConsumer() { git.execute(new LineConsumer() {

View File

@ -158,6 +158,12 @@ public class GitPreReceiveCallback extends HttpServlet {
if (commitMessageError != null) if (commitMessageError != null)
errorMessages.add(commitMessageError.toString()); errorMessages.add(commitMessageError.toString());
} }
if (errorMessages.isEmpty() && !protection.getDisallowedFileTypes().isEmpty()) {
var violatedFileTypes = protection.getViolatedFileTypes(project, oldObjectId, newObjectId, gitEnvs);
if (!violatedFileTypes.isEmpty()) {
errorMessages.add("Your push contains disallowed file type(s): " + StringUtils.join(violatedFileTypes, ", "));
}
}
if (errorMessages.isEmpty() if (errorMessages.isEmpty()
&& !oldObjectId.equals(ObjectId.zeroId()) && !oldObjectId.equals(ObjectId.zeroId())
&& !newObjectId.equals(ObjectId.zeroId()) && !newObjectId.equals(ObjectId.zeroId())
@ -202,6 +208,12 @@ public class GitPreReceiveCallback extends HttpServlet {
errorMessages.add("Can not update this tag as tag protection setting requires " errorMessages.add("Can not update this tag as tag protection setting requires "
+ "valid tag signature"); + "valid tag signature");
} }
if (errorMessages.isEmpty() && !protection.getDisallowedFileTypes().isEmpty()) {
var violatedFileTypes = protection.getViolatedFileTypes(project, newObjectId, gitEnvs);
if (!violatedFileTypes.isEmpty()) {
errorMessages.add("Your push contains disallowed file type(s): " + StringUtils.join(violatedFileTypes, ", "));
}
}
if (errorMessages.isEmpty()) { if (errorMessages.isEmpty()) {
for (var preReceiveChecker : preReceiveCheckers) { for (var preReceiveChecker : preReceiveCheckers) {
var errorMessage = preReceiveChecker.check(project, user, refName, oldObjectId, newObjectId); var errorMessage = preReceiveChecker.check(project, user, refName, oldObjectId, newObjectId);

View File

@ -148,7 +148,7 @@ public interface GitService {
@Nullable @Nullable
CommitMessageError checkCommitMessages(BranchProtection protection, Project project, CommitMessageError checkCommitMessages(BranchProtection protection, Project project,
ObjectId oldId, ObjectId newId, Map<String, String> envs); ObjectId oldId, ObjectId newId, Map<String, String> envs);
@Nullable @Nullable
byte[] getRawTag(Project project, ObjectId tagId, Map<String, String> envs); byte[] getRawTag(Project project, ObjectId tagId, Map<String, String> envs);

View File

@ -1607,6 +1607,12 @@ public class Project extends AbstractEntity implements LabelSupport<ProjectLabel
var protection = getBranchProtection(branch, user); var protection = getBranchProtection(branch, user);
return getGitService().checkCommitMessages(protection, this, oldCommitId, newCommitId, gitEnvs); return getGitService().checkCommitMessages(protection, this, oldCommitId, newCommitId, gitEnvs);
} }
public Collection<String> getViolatedFileTypes(String branch, User user, ObjectId oldCommitId, ObjectId newCommitId,
Map<String, String> gitEnvs) {
var protection = getBranchProtection(branch, user);
return protection.getViolatedFileTypes(this, oldCommitId, newCommitId, gitEnvs);
}
@Nullable @Nullable
public List<String> readLines(BlobIdent blobIdent, WhitespaceOption whitespaceOption, boolean mustExist) { public List<String> readLines(BlobIdent blobIdent, WhitespaceOption whitespaceOption, boolean mustExist) {

View File

@ -455,6 +455,8 @@ public class PullRequest extends ProjectBelonging
private transient Optional<CommitMessageError> commitMessageErrorOpt; private transient Optional<CommitMessageError> commitMessageErrorOpt;
private transient Optional<Collection<String>> violatedFileTypesOpt;
public String getTitle() { public String getTitle() {
return title; return title;
} }
@ -1197,7 +1199,10 @@ public class PullRequest extends ProjectBelonging
return _T("No valid signature for head commit"); return _T("No valid signature for head commit");
var commitMessageError = checkCommitMessages(); var commitMessageError = checkCommitMessages();
if (commitMessageError != null) if (commitMessageError != null)
return commitMessageError.toString(); return commitMessageError.toString();
var violatedFileTypes = getViolatedFileTypes();
if (!violatedFileTypes.isEmpty())
return MessageFormat.format(_T("Disallowed file type(s): {0}"), StringUtils.join(violatedFileTypes, ", "));
return null; return null;
} }
@ -1318,6 +1323,15 @@ public class PullRequest extends ProjectBelonging
return commitMessageErrorOpt.orElse(null); return commitMessageErrorOpt.orElse(null);
} }
public Collection<String> getViolatedFileTypes() {
if (violatedFileTypesOpt == null) {
var violatedFileTypes = getProject().getViolatedFileTypes(getTargetBranch(), getSubmitter(),
getBaseCommit().copy(), getLatestUpdate().getHeadCommit().copy(), new HashMap<>());
violatedFileTypesOpt = Optional.of(violatedFileTypes);
}
return violatedFileTypesOpt.get();
}
public static String getSerialLockName(Long requestId) { public static String getSerialLockName(Long requestId) {
return "pull-request-" + requestId + "-serial"; return "pull-request-" + requestId + "-serial";
} }

View File

@ -6,6 +6,7 @@ import static java.util.regex.Pattern.UNICODE_CHARACTER_CLASS;
import java.io.Serializable; import java.io.Serializable;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
@ -81,6 +82,8 @@ public class BranchProtection implements Serializable {
private List<String> commitTypesForFooterCheck = new ArrayList<>(); private List<String> commitTypesForFooterCheck = new ArrayList<>();
private Integer maxCommitMessageLineLength; private Integer maxCommitMessageLineLength;
private List<String> disallowedFileTypes = new ArrayList<>();
private String reviewRequirement; private String reviewRequirement;
@ -262,6 +265,15 @@ public class BranchProtection implements Serializable {
reviewRequirement = parsedReviewRequirement.toString(); reviewRequirement = parsedReviewRequirement.toString();
} }
@Editable(order=410, placeholder = "No disallowed file types", description = "Optionally specify disallowed file types by extensions (hit ENTER to add value), for instance <code>exe</code>, <code>bin</code>. Leave empty to allow all file types")
public List<String> getDisallowedFileTypes() {
return disallowedFileTypes;
}
public void setDisallowedFileTypes(List<String> disallowedFileTypes) {
this.disallowedFileTypes = disallowedFileTypes;
}
@Editable(order=500, name="Required Builds", placeholder="No any", description="Optionally choose required builds. You may also " + @Editable(order=500, name="Required Builds", placeholder="No any", description="Optionally choose required builds. You may also " +
"input jobs not listed here, and press ENTER to add them") "input jobs not listed here, and press ENTER to add them")
@JobChoice(tagsMode=true) @JobChoice(tagsMode=true)
@ -422,6 +434,18 @@ public class BranchProtection implements Serializable {
return false; return false;
} }
public Collection<String> getViolatedFileTypes(Project project, ObjectId oldObjectId, ObjectId newObjectId,
Map<String, String> gitEnvs) {
if (disallowedFileTypes.isEmpty()) {
return Collections.emptySet();
} else {
var changedFiles = getGitService().getChangedFiles(project, oldObjectId, newObjectId, gitEnvs);
return getDisallowedFileTypes().stream()
.filter(type -> changedFiles.stream().anyMatch(file -> file.toLowerCase().endsWith("." + type.toLowerCase())))
.collect(Collectors.toSet());
}
}
public BuildRequirement getBuildRequirement(Project project, ObjectId oldObjectId, ObjectId newObjectId, public BuildRequirement getBuildRequirement(Project project, ObjectId oldObjectId, ObjectId newObjectId,
Map<String, String> gitEnvs) { Map<String, String> gitEnvs) {
Collection<String> requiredJobs = new LinkedHashSet<>(getJobNames()); Collection<String> requiredJobs = new LinkedHashSet<>(getJobNames());

View File

@ -1,8 +1,22 @@
package io.onedev.server.model.support.code; package io.onedev.server.model.support.code;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import javax.validation.constraints.NotEmpty;
import org.eclipse.jgit.lib.ObjectId;
import io.onedev.commons.codeassist.InputSuggestion; import io.onedev.commons.codeassist.InputSuggestion;
import io.onedev.server.OneDev;
import io.onedev.server.annotation.Editable; import io.onedev.server.annotation.Editable;
import io.onedev.server.annotation.Patterns; import io.onedev.server.annotation.Patterns;
import io.onedev.server.git.service.GitService;
import io.onedev.server.model.Project; import io.onedev.server.model.Project;
import io.onedev.server.util.patternset.PatternSet; import io.onedev.server.util.patternset.PatternSet;
import io.onedev.server.util.usage.Usage; import io.onedev.server.util.usage.Usage;
@ -10,11 +24,6 @@ import io.onedev.server.util.usermatch.Anyone;
import io.onedev.server.util.usermatch.UserMatch; import io.onedev.server.util.usermatch.UserMatch;
import io.onedev.server.web.util.SuggestionUtils; import io.onedev.server.web.util.SuggestionUtils;
import javax.validation.constraints.NotEmpty;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
@Editable @Editable
public class TagProtection implements Serializable { public class TagProtection implements Serializable {
@ -33,6 +42,8 @@ public class TagProtection implements Serializable {
private boolean preventCreation = true; private boolean preventCreation = true;
private boolean commitSignatureRequired; private boolean commitSignatureRequired;
private List<String> disallowedFileTypes = new ArrayList<>();
public boolean isEnabled() { public boolean isEnabled() {
return enabled; return enabled;
@ -115,6 +126,15 @@ public class TagProtection implements Serializable {
this.commitSignatureRequired = commitSignatureRequired; this.commitSignatureRequired = commitSignatureRequired;
} }
@Editable(order=410, placeholder = "No disallowed file types", description = "Optionally specify disallowed file types by extensions (hit ENTER to add value), for instance <code>exe</code>, <code>bin</code>. Leave empty to allow all file types")
public List<String> getDisallowedFileTypes() {
return disallowedFileTypes;
}
public void setDisallowedFileTypes(List<String> disallowedFileTypes) {
this.disallowedFileTypes = disallowedFileTypes;
}
public void onRenameGroup(String oldName, String newName) { public void onRenameGroup(String oldName, String newName) {
userMatch = UserMatch.onRenameGroup(userMatch, oldName, newName); userMatch = UserMatch.onRenameGroup(userMatch, oldName, newName);
} }
@ -145,4 +165,19 @@ public class TagProtection implements Serializable {
return usage; return usage;
} }
public Collection<String> getViolatedFileTypes(Project project, ObjectId objectId, Map<String, String> gitEnvs) {
if (disallowedFileTypes.isEmpty()) {
return Collections.emptySet();
} else {
var files = getGitService().getChangedFiles(project, ObjectId.zeroId(), objectId, gitEnvs);
return getDisallowedFileTypes().stream()
.filter(type -> files.stream().anyMatch(file -> file.toLowerCase().endsWith("." + type.toLowerCase())))
.collect(Collectors.toSet());
}
}
private GitService getGitService() {
return OneDev.getInstance(GitService.class);
}
} }

View File

@ -4,8 +4,8 @@ requirement: WS* criteria (WS+ criteria)* WS* EOF;
criteria: userCriteria | groupCriteria; criteria: userCriteria | groupCriteria;
userCriteria: USER WS* Value; userCriteria: USER Value;
groupCriteria: GROUP WS* Value (WS*':' WS* DIGIT)?; groupCriteria: GROUP Value (':' DIGIT)?;
DIGIT: [1-9][0-9]*; DIGIT: [1-9][0-9]*;

View File

@ -1539,11 +1539,17 @@ public class ProjectBlobPage extends ProjectPage implements BlobRenderContext,
String blobPath = FilenameUtils.sanitizeFileName(FileUpload.getFileName(item)); String blobPath = FilenameUtils.sanitizeFileName(FileUpload.getFileName(item));
if (parentPath != null) if (parentPath != null)
blobPath = parentPath + "/" + blobPath; blobPath = parentPath + "/" + blobPath;
var blobType = StringUtils.substringAfterLast(blobPath, ".");
var disallowedFileTypes = getProject().getBranchProtection(blobIdent.revision, user).getDisallowedFileTypes();
if (disallowedFileTypes.stream().anyMatch(type -> blobType.equalsIgnoreCase(type))) {
throw new BlobEditException(MessageFormat.format(_T("Not allowed file type: {0}"), blobType));
}
if (getProject().isReviewRequiredForModification(user, blobIdent.revision, blobPath)) if (getProject().isReviewRequiredForModification(user, blobIdent.revision, blobPath))
throw new BlobEditException("Review required for this change. Please submit pull request instead"); throw new BlobEditException(_T("Review required for this change. Please submit pull request instead"));
else if (getProject().isBuildRequiredForModification(user, blobIdent.revision, blobPath)) else if (getProject().isBuildRequiredForModification(user, blobIdent.revision, blobPath))
throw new BlobEditException("Build required for this change. Please submit pull request instead"); throw new BlobEditException(_T("Build required for this change. Please submit pull request instead"));
else if (getProject().isCommitSignatureRequiredButNoSigningKey(user, blobIdent.revision)) else if (getProject().isCommitSignatureRequiredButNoSigningKey(user, blobIdent.revision))
signRequired = true; signRequired = true;

View File

@ -274,16 +274,25 @@ public class CommitOptionPanel extends Panel {
Map<String, BlobContent> newBlobs = new HashMap<>(); Map<String, BlobContent> newBlobs = new HashMap<>();
if (newContentProvider != null) { if (newContentProvider != null) {
String newPath = context.getNewPath(); String newPath = context.getNewPath();
var blobType = StringUtils.substringAfterLast(newPath, ".");
var disallowedFileTypes = context.getProject().getBranchProtection(revision, user).getDisallowedFileTypes();
if (disallowedFileTypes.stream().anyMatch(type -> blobType.equalsIgnoreCase(type))) {
form.error(MessageFormat.format(_T("Not allowed file type: {0}"), blobType));
target.add(form);
return false;
}
if (context.getProject().isReviewRequiredForModification(user, revision, newPath)) { if (context.getProject().isReviewRequiredForModification(user, revision, newPath)) {
form.error("Review required for this change. Please submit pull request instead"); form.error(_T("Review required for this change. Please submit pull request instead"));
target.add(form); target.add(form);
return false; return false;
} else if (context.getProject().isBuildRequiredForModification(user, revision, newPath)) { } else if (context.getProject().isBuildRequiredForModification(user, revision, newPath)) {
form.error("Build required for this change. Please submit pull request instead"); form.error(_T("Build required for this change. Please submit pull request instead"));
target.add(form); target.add(form);
return false; return false;
} else if (context.getProject().isCommitSignatureRequiredButNoSigningKey(user, revision)) { } else if (context.getProject().isCommitSignatureRequiredButNoSigningKey(user, revision)) {
form.error("Signature required for this change, but no signing key is specified"); form.error(_T("Signature required for this change, but no signing key is specified"));
target.add(form); target.add(form);
return false; return false;
} }
@ -307,13 +316,11 @@ public class CommitOptionPanel extends Panel {
ExceptionUtils.find(e, ObsoleteCommitException.class); ExceptionUtils.find(e, ObsoleteCommitException.class);
if (objectAlreadyExistsException != null) { if (objectAlreadyExistsException != null) {
form.error("A path with same name already exists. " form.error(_T("A path with same name already exists.Please choose a different name and try again."));
+ "Please choose a different name and try again.");
target.add(form); target.add(form);
break; break;
} else if (notTreeException != null) { } else if (notTreeException != null) {
form.error("A file exists where youre trying to create a subdirectory. " form.error(_T("A file exists where youre trying to create a subdirectory. Choose a new path and try again.."));
+ "Choose a new path and try again..");
target.add(form); target.add(form);
break; break;
} else if (obsoleteCommitException != null) { } else if (obsoleteCommitException != null) {
@ -333,14 +340,14 @@ public class CommitOptionPanel extends Panel {
if (newContentProvider != null) { if (newContentProvider != null) {
oldPaths.clear(); oldPaths.clear();
changesOfOthers = getBlobChange(path, pathChange, lastPrevCommitId, prevCommitId); changesOfOthers = getBlobChange(path, pathChange, lastPrevCommitId, prevCommitId);
form.warn("Someone made below change since you started editing"); form.warn(_T("Someone made below change since you started editing"));
break; break;
} else { } else {
newCommitId = obsoleteCommitException.getOldCommitId(); newCommitId = obsoleteCommitException.getOldCommitId();
} }
} else { } else {
changesOfOthers = getBlobChange(path, pathChange, lastPrevCommitId, prevCommitId); changesOfOthers = getBlobChange(path, pathChange, lastPrevCommitId, prevCommitId);
form.warn("Someone made below change since you started editing"); form.warn(_T("Someone made below change since you started editing"));
break; break;
} }
} }

View File

@ -94,6 +94,11 @@
<wicket:svg href="times-circle" class="icon"></wicket:svg> <span wicket:id="commitMessageCheckError"></span> <wicket:svg href="times-circle" class="icon"></wicket:svg> <span wicket:id="commitMessageCheckError"></span>
</div> </div>
</wicket:enclosure> </wicket:enclosure>
<wicket:enclosure child="fileTypesCheckError">
<div class="file-types-check-error text-warning">
<wicket:svg href="times-circle" class="icon"></wicket:svg> <span wicket:id="fileTypesCheckError"></span>
</div>
</wicket:enclosure>
<wicket:enclosure child="requiredJobs"> <wicket:enclosure child="requiredJobs">
<div class="required-jobs text-warning"> <div class="required-jobs text-warning">
<wicket:svg href="warning" class="icon"></wicket:svg> <wicket:svg href="warning" class="icon"></wicket:svg>

View File

@ -724,6 +724,27 @@ public abstract class PullRequestDetailPage extends ProjectPage implements PullR
} }
}.setEscapeModelStrings(false)); }.setEscapeModelStrings(false));
summaryContainer.add(new Label("fileTypesCheckError", new LoadableDetachableModel<>() {
@Override
protected String load() {
var violatedFileTypes = getPullRequest().getViolatedFileTypes();
if (violatedFileTypes.isEmpty())
return null;
else
return MessageFormat.format(_T("The change contains disallowed file type(s): {0}"), StringUtils.join(violatedFileTypes, ", "));
}
}) {
@Override
protected void onConfigure() {
super.onConfigure();
PullRequest request = getPullRequest();
if (request.isOpen())
setVisible(getDefaultModelObject() != null);
else
setVisible(false);
}
}.setEscapeModelStrings(false));
summaryContainer.add(new Label("requiredJobsMessage", new AbstractReadOnlyModel<String>() { summaryContainer.add(new Label("requiredJobsMessage", new AbstractReadOnlyModel<String>() {
@Override @Override
public String getObject() { public String getObject() {