Skip to content

Commit 8f7ad86

Browse files
committed
Revert "Investigation into fetching from remote repo with JGit"
This reverts commit 49f3f37.
1 parent 49f3f37 commit 8f7ad86

File tree

5 files changed

+40
-96
lines changed

5 files changed

+40
-96
lines changed

src/main/java/org/commonwl/view/cwl/CWLService.java

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.fasterxml.jackson.databind.node.ArrayNode;
2525
import com.fasterxml.jackson.databind.node.ObjectNode;
2626
import com.fasterxml.jackson.databind.node.TextNode;
27+
import org.apache.commons.io.FileUtils;
2728
import org.apache.commons.io.FilenameUtils;
2829
import org.apache.jena.ontology.OntModelSpec;
2930
import org.apache.jena.query.QuerySolution;
@@ -35,24 +36,26 @@
3536
import org.apache.jena.riot.RiotException;
3637
import org.commonwl.view.docker.DockerService;
3738
import org.commonwl.view.git.GitDetails;
38-
import org.commonwl.view.git.GitService;
3939
import org.commonwl.view.graphviz.ModelDotWriter;
4040
import org.commonwl.view.graphviz.RDFDotWriter;
4141
import org.commonwl.view.workflow.Workflow;
42-
import org.eclipse.jgit.errors.LargeObjectException;
4342
import org.slf4j.Logger;
4443
import org.slf4j.LoggerFactory;
4544
import org.springframework.beans.factory.annotation.Autowired;
45+
import org.springframework.beans.factory.annotation.Value;
4646
import org.springframework.stereotype.Service;
4747
import org.yaml.snakeyaml.Yaml;
4848

4949
import java.io.ByteArrayInputStream;
50+
import java.io.File;
5051
import java.io.IOException;
5152
import java.io.StringWriter;
5253
import java.nio.file.Path;
5354
import java.nio.file.Paths;
5455
import java.util.*;
5556

57+
import static org.apache.commons.io.FileUtils.readFileToString;
58+
5659
/**
5760
* Provides CWL parsing for workflows to gather an overview
5861
* for display and visualisation
@@ -65,7 +68,7 @@ public class CWLService {
6568
// Autowired properties/services
6669
private final RDFService rdfService;
6770
private final CWLTool cwlTool;
68-
private final GitService gitService;
71+
private final int singleFileSizeLimit;
6972

7073
// CWL specific strings
7174
private final String DOC_GRAPH = "$graph";
@@ -95,27 +98,30 @@ public class CWLService {
9598
* Constructor for the Common Workflow Language service
9699
* @param rdfService A service for handling RDF queries
97100
* @param cwlTool Handles cwltool integration
98-
* @param gitService Handles Git repository functionality
101+
* @param singleFileSizeLimit The file size limit for single files
99102
*/
100103
@Autowired
101104
public CWLService(RDFService rdfService,
102105
CWLTool cwlTool,
103-
GitService gitService) {
106+
@Value("${singleFileSizeLimit}") int singleFileSizeLimit) {
104107
this.rdfService = rdfService;
105108
this.cwlTool = cwlTool;
106-
this.gitService = gitService;
109+
this.singleFileSizeLimit = singleFileSizeLimit;
107110
}
108111

109112
/**
110113
* Gets the Workflow object from internal parsing
111-
* @param gitDetails The details for the workflow in the git repository
114+
* @param workflowFile The workflow file to be parsed
112115
* @return The constructed workflow object
113116
*/
114-
public Workflow parseWorkflowNative(GitDetails gitDetails) throws IOException {
115-
try {
117+
public Workflow parseWorkflowNative(File workflowFile) throws IOException {
118+
119+
// Check file size limit before parsing
120+
long fileSizeBytes = workflowFile.length();
121+
if (fileSizeBytes <= singleFileSizeLimit) {
122+
116123
// Parse file as yaml
117-
JsonNode cwlFile = yamlStringToJson(gitService.getFile(gitService
118-
.getRepository(gitDetails).getRepository(), gitDetails.getPath()));
124+
JsonNode cwlFile = yamlStringToJson(readFileToString(workflowFile));
119125

120126
// If the CWL file is packed there can be multiple workflows in a file
121127
Map<String, JsonNode> packedFiles = new HashMap<>();
@@ -132,7 +138,7 @@ public Workflow parseWorkflowNative(GitDetails gitDetails) throws IOException {
132138
// Use filename for label if there is no defined one
133139
String label = extractLabel(cwlFile);
134140
if (label == null) {
135-
label = FilenameUtils.getName(gitDetails.getPath());
141+
label = FilenameUtils.getName(workflowFile.getPath());
136142
}
137143

138144
// Construct the rest of the workflow model
@@ -157,19 +163,22 @@ public Workflow parseWorkflowNative(GitDetails gitDetails) throws IOException {
157163

158164
return workflowModel;
159165

160-
} catch (LargeObjectException ex) {
161-
throw new IOException("File '" + FilenameUtils.getName(gitDetails.getPath()) +
162-
"' is over singleFileSizeLimit";
166+
} else {
167+
throw new IOException("File '" + workflowFile.getName() + "' is over singleFileSizeLimit - " +
168+
FileUtils.byteCountToDisplaySize(fileSizeBytes) + "/" +
169+
FileUtils.byteCountToDisplaySize(singleFileSizeLimit));
163170
}
164171

165172
}
166173

167174
/**
168175
* Create a workflow model using cwltool rdf output
169176
* @param basicModel The basic workflow object created thus far
177+
* @param workflowFile The workflow file to run cwltool on
170178
* @return The constructed workflow object
171179
*/
172-
public Workflow parseWorkflowWithCwltool(Workflow basicModel) throws CWLValidationException {
180+
public Workflow parseWorkflowWithCwltool(Workflow basicModel,
181+
File workflowFile) throws CWLValidationException {
173182
GitDetails gitDetails = basicModel.getRetrievedFrom();
174183
String latestCommit = basicModel.getLastCommit();
175184
String packedWorkflowID = basicModel.getPackedWorkflowID();

src/main/java/org/commonwl/view/cwl/CWLToolRunner.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.springframework.scheduling.annotation.EnableAsync;
3232
import org.springframework.stereotype.Component;
3333

34+
import java.io.File;
3435
import java.io.IOException;
3536
import java.util.Date;
3637

@@ -63,14 +64,16 @@ public CWLToolRunner(WorkflowRepository workflowRepository,
6364
}
6465

6566
@Async
66-
public void createWorkflowFromQueued(QueuedWorkflow queuedWorkflow)
67+
public void createWorkflowFromQueued(QueuedWorkflow queuedWorkflow, File workflowFile)
6768
throws IOException, InterruptedException {
6869

6970
Workflow tempWorkflow = queuedWorkflow.getTempRepresentation();
7071

7172
// Parse using cwltool and replace in database
7273
try {
73-
Workflow newWorkflow = cwlService.parseWorkflowWithCwltool(tempWorkflow);
74+
Workflow newWorkflow = cwlService.parseWorkflowWithCwltool(
75+
tempWorkflow,
76+
workflowFile);
7477

7578
// Success
7679
newWorkflow.setRetrievedFrom(tempWorkflow.getRetrievedFrom());

src/main/java/org/commonwl/view/git/GitService.java

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,8 @@
2323
import org.commonwl.view.researchobject.HashableAgent;
2424
import org.eclipse.jgit.api.Git;
2525
import org.eclipse.jgit.api.errors.GitAPIException;
26-
import org.eclipse.jgit.lib.ObjectId;
27-
import org.eclipse.jgit.lib.ObjectLoader;
2826
import org.eclipse.jgit.lib.PersonIdent;
29-
import org.eclipse.jgit.lib.Repository;
3027
import org.eclipse.jgit.revwalk.RevCommit;
31-
import org.eclipse.jgit.revwalk.RevTree;
32-
import org.eclipse.jgit.revwalk.RevWalk;
33-
import org.eclipse.jgit.treewalk.TreeWalk;
34-
import org.eclipse.jgit.treewalk.filter.PathFilter;
3528
import org.slf4j.Logger;
3629
import org.slf4j.LoggerFactory;
3730
import org.springframework.beans.factory.annotation.Autowired;
@@ -60,16 +53,11 @@ public class GitService {
6053
// Whether submodules are also cloned
6154
private boolean cloneSubmodules;
6255

63-
// File size limit for loading in bytes
64-
private int singleFileSizeLimit;
65-
6656
@Autowired
6757
public GitService(@Value("${gitStorage}") Path gitStorage,
68-
@Value("${gitAPI.cloneSubmodules}") boolean cloneSubmodules,
69-
@Value("${singleFileSizeLimit}") int singleFileSizeLimit) {
58+
@Value("${gitAPI.cloneSubmodules}") boolean cloneSubmodules) {
7059
this.gitStorage = gitStorage;
7160
this.cloneSubmodules = cloneSubmodules;
72-
this.singleFileSizeLimit = singleFileSizeLimit;
7361
}
7462

7563
/**
@@ -120,46 +108,6 @@ public Git getRepository(GitDetails gitDetails)
120108
return repo;
121109
}
122110

123-
/**
124-
* Get the contents of a file from the Git repository
125-
* @param repository The Git repository
126-
* @param refOrCommitId The branch name or commit ID as a string
127-
* @return The contents of the file as a string
128-
* @throws IOException Any errors in retrieving the file
129-
*/
130-
public String getFile(Repository repository, String refOrCommitId) throws IOException {
131-
String content;
132-
133-
// Get the ObjectID from the ref or commit ID string
134-
if (!ObjectId.isId(refOrCommitId)) {
135-
refOrCommitId = "refs/remotes/origin/" + refOrCommitId;
136-
}
137-
ObjectId commitId = repository.resolve(refOrCommitId);
138-
139-
// Walk over commits using defined filtering
140-
try (RevWalk revWalk = new RevWalk(repository)) {
141-
RevCommit commit = revWalk.parseCommit(commitId);
142-
RevTree tree = commit.getTree();
143-
144-
// Try to find specific file in the repository
145-
try (TreeWalk treeWalk = new TreeWalk(repository)) {
146-
treeWalk.addTree(tree);
147-
treeWalk.setRecursive(true);
148-
treeWalk.setFilter(PathFilter.create("README.md"));
149-
if (!treeWalk.next()) {
150-
throw new IllegalStateException("Did not find expected file");
151-
}
152-
153-
ObjectId objectId = treeWalk.getObjectId(0);
154-
ObjectLoader loader = repository.open(objectId);
155-
content = new String(loader.getCachedBytes(singleFileSizeLimit));
156-
}
157-
revWalk.dispose();
158-
}
159-
160-
return content;
161-
}
162-
163111
/**
164112
* Gets the commit ID of the HEAD for the given repository
165113
* @param repo The Git repository

src/main/java/org/commonwl/view/workflow/WorkflowService.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,14 @@ public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
236236
File localPath = repo.getRepository().getWorkTree();
237237
String latestCommit = gitService.getCurrentCommitID(repo);
238238

239-
Workflow basicModel = cwlService.parseWorkflowNative(gitInfo);
239+
Path pathToWorkflowFile = localPath.toPath().resolve(gitInfo.getPath()).normalize().toAbsolutePath();
240+
// Prevent path traversal attacks
241+
if (!pathToWorkflowFile.startsWith(localPath.toPath().normalize().toAbsolutePath())) {
242+
throw new WorkflowNotFoundException();
243+
}
244+
245+
File workflowFile = new File(pathToWorkflowFile.toString());
246+
Workflow basicModel = cwlService.parseWorkflowNative(workflowFile);
240247

241248
// Set origin details
242249
basicModel.setRetrievedOn(new Date());
@@ -251,7 +258,7 @@ public QueuedWorkflow createQueuedWorkflow(GitDetails gitInfo)
251258
// ASYNC OPERATIONS
252259
// Parse with cwltool and update model
253260
try {
254-
cwlToolRunner.createWorkflowFromQueued(queuedWorkflow);
261+
cwlToolRunner.createWorkflowFromQueued(queuedWorkflow, workflowFile);
255262
} catch (Exception e) {
256263
logger.error("Could not update workflow with cwltool", e);
257264
}

src/test/java/org/commonwl/view/git/GitServiceTest.java

Lines changed: 0 additions & 23 deletions
This file was deleted.

0 commit comments

Comments
 (0)