diff --git a/core/src/test/java/apoc/export/ExportCoreSecurityTest.java b/core/src/test/java/apoc/export/ExportCoreSecurityTest.java index 231c8ce99..74a91875f 100644 --- a/core/src/test/java/apoc/export/ExportCoreSecurityTest.java +++ b/core/src/test/java/apoc/export/ExportCoreSecurityTest.java @@ -25,7 +25,7 @@ import apoc.export.json.ExportJson; import apoc.util.FileUtils; import apoc.util.TestUtil; -import junit.framework.TestCase; +import com.nimbusds.jose.util.Pair; import org.junit.Assert; import org.junit.BeforeClass; import org.junit.ClassRule; @@ -34,9 +34,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.neo4j.configuration.GraphDatabaseSettings; -import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.QueryExecutionException; -import org.neo4j.graphdb.Result; import org.neo4j.test.rule.DbmsRule; import org.neo4j.test.rule.ImpermanentDbmsRule; @@ -49,18 +47,26 @@ import java.util.function.Consumer; import java.util.stream.Collectors; +import static apoc.export.SecurityTestUtil.ERROR_KEY; +import static apoc.export.SecurityTestUtil.EXPORT_PROCEDURES; +import static apoc.export.SecurityTestUtil.PROCEDURE_KEY; +import static apoc.export.SecurityTestUtil.assertPathTraversalError; +import static apoc.export.SecurityTestUtil.getApocProcedure; +import static apoc.export.SecurityTestUtil.setExportFileApocConfigs; import static apoc.util.TestUtil.assertError; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; @RunWith(Enclosed.class) public class ExportCoreSecurityTest { - private static final File directory = new File("target/import"); private static final File directoryWithSamePrefix = new File("target/imported"); private static final File subDirectory = new File("target/import/tests"); private static final List APOC_EXPORT_PROCEDURE_NAME = Arrays.asList("csv", "json", "graphml", "cypher"); + public static final String FILENAME = "my-test.txt"; + public static final String PARAM_NAMES = "Procedure: {0}.{1}, fileName: {3}"; + + static { //noinspection ResultOfMethodCallIgnored directory.mkdirs(); @@ -77,411 +83,291 @@ public class ExportCoreSecurityTest { @BeforeClass public static void setUp() { TestUtil.registerProcedure(db, ExportCSV.class, ExportJson.class, ExportGraphML.class, ExportCypher.class); - setFileExport(false); } public static void setFileExport(boolean allowed) { ApocConfig.apocConfig().setProperty(ApocConfig.APOC_EXPORT_FILE_ENABLED, allowed); } - private static Collection data(Map> apocProcedureArguments) { - return data(apocProcedureArguments, APOC_EXPORT_PROCEDURE_NAME); + private static List getParameterData(List>> fileAndErrors) { + return getParameterData(fileAndErrors, EXPORT_PROCEDURES, APOC_EXPORT_PROCEDURE_NAME); } - public static Collection data(Map> apocProcedureArguments, List procedureNames) { - return procedureNames - .stream() - .flatMap(method -> apocProcedureArguments - .entrySet() + public static List getParameterData(List>> fileAndErrors, List> importAndLoadProcedures, List procedureNames) { + // from a stream of fileNames and a List of Pair + // returns a List of String[]{ procName, procArgs, fileName } + return fileAndErrors.stream() + .flatMap(fileName -> importAndLoadProcedures .stream() - .flatMap(e -> e.getValue() + .flatMap(procPair -> procedureNames .stream() - .map(a -> new String[]{method, e.getKey(), a}))) - .collect(Collectors.toList()); + .map(procName -> + new Object[] { procName, procPair.getLeft(), procPair.getRight(), fileName.getLeft(), fileName.getRight() } + ) + ) + ).toList(); } + /** + * These test with `apoc.import.file.use_neo4j_config=true` + * should fail because they access a directory which isn't the configured directory + */ @RunWith(Parameterized.class) - public static class TestIllegalFSAccess { - private final String apocProcedure; - - public TestIllegalFSAccess(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); - } + public static class TestIllegalExternalFSAccess { - private static final Map> APOC_PROCEDURE_ARGUMENTS = Map.of( - "query", List.of( - "\"RETURN 'hello' as key\", './hello', {}", - "\"RETURN 'hello' as key\", './hello', {stream:true}", - "\"RETURN 'hello' as key\", ' ', {}" - ), - "all", List.of( - "'./hello', {}", - "'./hello', {stream:true}", - "' ', {}" - ), - "data", List.of( - "[], [], './hello', {}", - "[], [], './hello', {stream:true}", - "[], [], ' ', {}" - ), - "graph", List.of( - "{nodes: [], relationships: []}, './hello', {}", - "{nodes: [], relationships: []}, './hello', {stream:true}", - "{nodes: [], relationships: []}, ' ', {}" - ) + // these 2 consumers accept a Map.of("error", , "procedure", ) + public static final Consumer EXCEPTION_OUTDIR_CONSUMER = (Map e) -> assertError( + (Exception) e.get(ERROR_KEY), FileUtils.ACCESS_OUTSIDE_DIR_ERROR, IOException.class, (String) e.get(PROCEDURE_KEY) + ); + public static final Consumer EXCEPTION_NOT_FOUND_CONSUMER = (Map e) -> assertTrue( + ((Exception) e.get(ERROR_KEY)).getMessage().contains("test.txt (No such file or directory)") ); - @Parameterized.Parameters - public static Collection data() { - return ExportCoreSecurityTest.data(APOC_PROCEDURE_ARGUMENTS); - } - - @Test - public void testIllegalFSAccessExport() { - QueryExecutionException e = Assert.assertThrows(QueryExecutionException.class, - () -> TestUtil.testCall(db, "CALL " + apocProcedure, (r) -> {}) - ); - - assertError(e, ApocConfig.EXPORT_TO_FILE_ERROR, RuntimeException.class, apocProcedure); - } - } - - @RunWith(Parameterized.class) - public static class TestIllegalExternalFSAccess { private final String apocProcedure; + private final String fileName; + private final Consumer consumer; - public TestIllegalExternalFSAccess(String exportMethod, String exportMethodType, String exportMethodArguments) { + public TestIllegalExternalFSAccess(String exportMethod, String exportMethodType, String exportMethodArguments, + String fileName, + Consumer consumer) { this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); + this.fileName = fileName; + this.consumer = consumer; } - private static final Map> METHOD_ARGUMENTS = Map.of( - "query", List.of( - "\"RETURN 'hello' as key\", '../hello', {}", - "\"RETURN 'hello' as key\", 'file:../hello', {}" - ), - "all", List.of( - "'../hello', {}", - "'file:../hello', {}" - ), - "data", List.of( - "[], [], '../hello', {}", - "[], [], 'file:../hello', {}" - ), - "graph", List.of( - "{nodes: [], relationships: []}, '../hello', {}", - "{nodes: [], relationships: []}, 'file:../hello', {}" - ) - ); + /* + * These test cases attempt to access a directory with the same prefix as the import directory. This is design to + * test "directoryName.startsWith" logic which is a common path traversal bug. + * All these tests should fail because they access a directory which isn't the configured directory + */ + private static final String case1 = "../imported/" + FILENAME; + private static final String case2 = "tests/../../imported/" + FILENAME; + private static final String case3 = "../" + FILENAME; + private static final String case4 = "file:../" + FILENAME; + private static final String case5 = "file:..//" + FILENAME; + + // non-failing cases, with apoc.import.file.use_neo4j_config=false + public static final List casesAllowed = Arrays.asList(case3, case4, case5); + + private static final String case16 = "file:///%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2fapoc/" + FILENAME; + public static final String case26 = "file:///%2e%2e%2f%2f" + FILENAME; + private static final String case46 = "tests/../../" + FILENAME; + private static final String case56 = "tests/..//..//" + FILENAME; + + public static final List casesOutsideDir = Arrays.asList(case1, case2, case3, case4, case5, + case16, case26, case46, case56); + + /* + All of these will resolve to a local path after normalization which will point to + a non-existing directory in our import folder: /apoc. Causing them to error that is + not found. They all attempt to exit the import folder back to the apoc folder: + Directory Layout: .../apoc/core/target/import + */ + private static final String case111 = "file://%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f/apoc/" + FILENAME; + private static final String case211 = "file://../../../../apoc/" + FILENAME; + private static final String case311 = "file:///..//..//..//..//apoc//core//..//" + FILENAME; + private static final String case411 = "file:///..//..//..//..//apoc/" + FILENAME; + private static final String case511 = "file://" + directory.getAbsolutePath() + "//..//..//..//..//apoc/" + FILENAME; + private static final String case611 = "file:///%252e%252e%252f%252e%252e%252f%252e%252e%252f%252e%252e%252f/apoc/" + FILENAME; + + public static final List casesNotExistingDir = Arrays.asList(case111, case211, case311, case411, case511, case611); + + public static List>> dataPairs; + + static { + dataPairs = casesOutsideDir.stream() + .map(i -> Pair.of(i, EXCEPTION_OUTDIR_CONSUMER)) + .collect(Collectors.toList()); + + List>> notExistingDirList = casesNotExistingDir.stream() + .map(i -> Pair.of(i, EXCEPTION_NOT_FOUND_CONSUMER)) + .toList(); + + dataPairs.addAll(notExistingDirList); + } - @Parameterized.Parameters - public static Collection data() { - return ExportCoreSecurityTest.data(METHOD_ARGUMENTS); + @Parameterized.Parameters(name = PARAM_NAMES) + public static Collection data() { + return ExportCoreSecurityTest.getParameterData(dataPairs); } @Test - public void testIllegalExternalFSAccessExport() { - final String message = apocProcedure + " should throw an exception"; - try { - setFileExport(true); - db.executeTransactionally("CALL " + apocProcedure, Map.of(), - Result::resultAsString); - fail(message); - } catch (Exception e) { - assertError(e, FileUtils.ACCESS_OUTSIDE_DIR_ERROR, IOException.class, apocProcedure); - } finally { - setFileExport(false); - } + public void testsWithExportDisabled() { + SecurityTestUtil.testsWithExportDisabled(db, apocProcedure, fileName); } - } - @RunWith(Parameterized.class) - public static class TestPathTraversalAccess { - private final String apocProcedure; - - public TestPathTraversalAccess(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); - } + @Test + public void testIllegalExternalFSAccessExportWithExportAndUseNeo4jConfEnabled() { + // all assertions with `apoc.export.file.enabled=true` and `apoc.import.file.use_neo4j_config=true` - private static final String case1 = "'file:///%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2fapoc/test.txt'"; - private static final String case2 = "'file:///%2e%2e%2f%2ftest.txt'"; - private static final String case3 = "'../test.txt'"; - private static final String case4 = "'tests/../../test.txt'"; - private static final String case5 = "'tests/..//..//test.txt'"; - - public static final List cases = Arrays.asList(case1, case2, case3, case4, case5); - - private static final Map> METHOD_ARGUMENTS = Map.of( - "query", cases.stream().map( - filePath -> "\"RETURN 1\", " + filePath + ", {}" - ).toList(), - "all", cases.stream().map( - filePath -> filePath + ", {}" - ).toList(), - "data", cases.stream().map( - filePath -> "[], [], " + filePath + ", {}" - ).toList(), - "graph", cases.stream().map( - filePath -> "{nodes: [], relationships: []}, " + filePath + ", {}" - ).toList() - ); + // apoc.import.file.allow_read_from_filesystem=false + setExportFileApocConfigs(true, true, false); + assertPathTraversalError(db, apocProcedure, Map.of("fileName", fileName), consumer); - @Parameterized.Parameters - public static Collection data() { - return ExportCoreSecurityTest.data(METHOD_ARGUMENTS); + // apoc.import.file.allow_read_from_filesystem=true + setExportFileApocConfigs(true, true, true); + assertPathTraversalError(db, apocProcedure, Map.of("fileName", fileName), consumer); } @Test - public void testPathTraversal() { - assertPathTraversalError(db, "CALL " + apocProcedure); - } - } + public void testWithUseNeo4jConfDisabled() { + // all assertions with `apoc.export.file.enabled=true` and `apoc.import.file.use_neo4j_config=false` - /** - * All of these will resolve to a local path after normalization which will point to - * a non-existing directory in our import folder: /apoc. Causing them to error that is - * not found. They all attempt to exit the import folder back to the apoc folder: - * Directory Layout: .../apoc/core/target/import - */ - @RunWith(Parameterized.class) - public static class TestPathTraversalIsNormalised { - private final String apocProcedure; + // apoc.import.file.allow_read_from_filesystem=true + setExportFileApocConfigs(true, false, true); + testWithUseNeo4jConfFalse(); - public TestPathTraversalIsNormalised(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); + // apoc.import.file.allow_read_from_filesystem=false + setExportFileApocConfigs(true, false, false); + testWithUseNeo4jConfFalse(); } - private static final String case1 = "'file://%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f/apoc/test.txt'"; - private static final String case2 = "'file://../../../../apoc/test.txt'"; - private static final String case3 = "'file:///..//..//..//..//apoc//core//..//test.txt'"; - private static final String case4 = "'file:///..//..//..//..//apoc/test.txt'"; - private static final String case5 = "'file://" + directory.getAbsolutePath() + "//..//..//..//..//apoc/test.txt'"; - private static final String case6 = "'file:///%252e%252e%252f%252e%252e%252f%252e%252e%252f%252e%252e%252f/apoc/test.txt'"; - - public static final List cases = Arrays.asList(case1, case2, case3, case4, case5, case6); - - private static final Map> METHOD_ARGUMENTS = Map.of( - "query", cases.stream().map( - filePath -> "\"RETURN 1\", " + filePath + ", {}" - ).toList(), - "all", cases.stream().map( - filePath -> filePath + ", {}" - ).toList(), - "data", cases.stream().map( - filePath -> "[], [], " + filePath + ", {}" - ).toList(), - "graph", cases.stream().map( - filePath -> "{nodes: [], relationships: []}, " + filePath + ", {}" - ).toList() - ); + private void testWithUseNeo4jConfFalse() { + // with `apoc.import.file.use_neo4j_config=false` this file export could outside the project + if (fileName.equals(case26)) { + return; + } - @Parameterized.Parameters - public static Collection data() { - return ExportCoreSecurityTest.data(METHOD_ARGUMENTS); + if (casesAllowed.contains(fileName)) { + assertPathTraversalWithoutErrors(); + } else { + assertPathTraversalError(db, apocProcedure, Map.of("fileName", fileName), EXCEPTION_NOT_FOUND_CONSUMER); + } } - @Test - public void testPathTraversal() { - assertPathTraversalError(db, "CALL " + apocProcedure, - e -> TestCase.assertTrue(e.getMessage().contains("apoc/test.txt (No such file or directory)")) - ); + private void assertPathTraversalWithoutErrors() { + SecurityTestUtil.assertPathTraversalWithoutErrors(db, apocProcedure, fileName, new File("../", FILENAME)); } } + /** - * These tests normalize the path to be within the import directory and make the file there. + * These tests normalize the path to be within the import directory (or subdirectory) and make the file there. * Some attempt to exit the directory. - * They result in a file name test.txt being created (and deleted after). */ @RunWith(Parameterized.class) public static class TestPathTraversalIsNormalisedWithinDirectory { + + public static final Consumer MAIN_DIR_CONSUMER = (r) -> assertTrue(((String) r.get("file")).contains("" + FILENAME)); + public static final Consumer SUB_DIR_CONSUMER = (r) -> assertTrue(((String) r.get("file")).contains("tests/" + FILENAME)); + private final String apocProcedure; + private final String fileName; + private final Consumer consumer; - public TestPathTraversalIsNormalisedWithinDirectory(String exportMethod, String exportMethodType, String exportMethodArguments) { + public TestPathTraversalIsNormalisedWithinDirectory(String exportMethod, String exportMethodType, String exportMethodArguments, + String fileName, + Consumer consumer) { this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); + this.fileName = fileName; + this.consumer = consumer; } - private static final String case1 = "'file:///..//..//..//..//apoc//..//..//..//..//test.txt'"; - private static final String case2 = "'file:///..//..//..//..//apoc//..//test.txt'"; - private static final String case3 = "'file:///../import/../import//..//test.txt'"; - private static final String case4 = "'file://test.txt'"; - private static final String case5 = "'file://tests/../test.txt'"; - private static final String case6 = "'file:///tests//..//test.txt'"; - private static final String case7 = "'test.txt'"; - private static final String case8 = "'file:///..//..//..//..//test.txt'"; - - public static final List cases = Arrays.asList(case1, case2, case3, case4, case5, case6, case7, case8); - - private static final Map> METHOD_ARGUMENTS = Map.of( - "query", cases.stream().map( - filePath -> "\"RETURN 1\", " + filePath + ", {}" - ).toList(), - "all", cases.stream().map( - filePath -> filePath + ", {}" - ).toList(), - "data", cases.stream().map( - filePath -> "[], [], " + filePath + ", {}" - ).toList(), - "graph", cases.stream().map( - filePath -> "{nodes: [], relationships: []}, " + filePath + ", {}" - ).toList() - ); - - @Parameterized.Parameters - public static Collection data() { - return ExportCoreSecurityTest.data(METHOD_ARGUMENTS); + /* + These tests normalize the path to be within the import directory and make the file there. + They result in a file being created (and deleted after). + */ + private static final String case0 = "./" + FILENAME; + + private static final String case1 = "file:///..//..//..//..//apoc//..//..//..//..//" + FILENAME; + private static final String case2 = "file:///..//..//..//..//apoc//..//" + FILENAME; + private static final String case3 = "file:///../import/../import//..//" + FILENAME; + private static final String case4 = "file://" + FILENAME; + private static final String case5 = "file://tests/../" + FILENAME; + private static final String case6 = "file:///tests//..//" + FILENAME; + private static final String case7 = "" + FILENAME; + private static final String case8 = "file:///..//..//..//..//" + FILENAME; + + public static final List mainDirCases = Arrays.asList(case0, case1, case2, case3, case4, case5, case6, case7, case8); + + /* + These tests normalize the path to be within the import directory and step into a subdirectory + to make the file there. + They result in a file in the directory /tests being created (and deleted after). + */ + private static final String case16 = "file:///../import/../import//..//tests/" + FILENAME; + private static final String case26 = "file:///..//..//..//..//apoc//..//tests/" + FILENAME; + private static final String case36 = "file:///../import/../import//..//tests/../tests/" + FILENAME; + private static final String case46 = "file:///tests/" + FILENAME; + private static final String case56 = "tests/" + FILENAME; + + public static final List subDirCases = Arrays.asList(case16, case26, case36, case46, case56); + + + @Parameterized.Parameters(name = PARAM_NAMES) + public static Collection data() { + List>> collect = mainDirCases.stream().map(i -> Pair.of(i, MAIN_DIR_CONSUMER)).collect(Collectors.toList()); + List>> collect2 = subDirCases.stream().map(i -> Pair.of(i, SUB_DIR_CONSUMER)).toList(); + collect.addAll(collect2); + + return getParameterData(collect); } @Test public void testPathTraversal() { - assertPathTraversalWithoutErrors(db, apocProcedure, directory, - (r) -> assertTrue(((String) r.get("file")).contains("test.txt"))); + File dir = getDir(); + setExportFileApocConfigs(true, true, false); + assertPathTraversalWithoutErrors(dir); } - } - /* - * These test cases attempt to access a directory with the same prefix as the import directory. This is design to - * test "directoryName.startsWith" logic which is a common path traversal bug. - * - * All these tests should fail because they access a directory which isn't the configured directory - */ - @RunWith(Parameterized.class) - public static class TestPathTraversalIsWithSimilarDirectoryName { - private final String apocProcedure; - - public TestPathTraversalIsWithSimilarDirectoryName(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); - } - - private static final String case1 = "'../imported/test.txt'"; - private static final String case2 = "'tests/../../imported/test.txt'"; - - public static final List cases = Arrays.asList(case1, case2); - - private static final Map> METHOD_ARGUMENTS = Map.of( - "query", cases.stream().map( - filePath -> "\"RETURN 1\", " + filePath + ", {}" - ).toList(), - "all", cases.stream().map( - filePath -> filePath + ", {}" - ).toList(), - "data", cases.stream().map( - filePath -> "[], [], " + filePath + ", {}" - ).toList(), - "graph", cases.stream().map( - filePath -> "{nodes: [], relationships: []}, " + filePath + ", {}" - ).toList() - ); - - @Parameterized.Parameters - public static Collection data() { - return ExportCoreSecurityTest.data(METHOD_ARGUMENTS); + @Test + public void testIllegalFSAccessExport() { + SecurityTestUtil.testsWithExportDisabled(db, apocProcedure, fileName); } @Test - public void testPathTraversal() { - assertPathTraversalError(db, "CALL " + apocProcedure); - } - } + public void testWithUseNeo4jConfDisabled() { + // all assertions with `apoc.export.file.enabled=true` and `apoc.import.file.use_neo4j_config=true` + File dir = getDir(); - /** - * These tests normalize the path to be within the import directory and step into a subdirectory - * to make the file there. - * Some attempt to exit the directory. - * They result in a file name test.txt in the directory /tests being created (and deleted after). - */ - @RunWith(Parameterized.class) - public static class TestPathTraversAllowedWithinDirectory { - private final String apocProcedure; + // apoc.import.file.allow_read_from_filesystem=false + setExportFileApocConfigs(true, true, false); + assertPathTraversalWithoutErrors(dir); - public TestPathTraversAllowedWithinDirectory(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); + // apoc.import.file.allow_read_from_filesystem=true + setExportFileApocConfigs(true, true, true); + assertPathTraversalWithoutErrors(dir); } - private static final String case1 = "'file:///../import/../import//..//tests/test.txt'"; - private static final String case2 = "'file:///..//..//..//..//apoc//..//tests/test.txt'"; - private static final String case3 = "'file:///../import/../import//..//tests/../tests/test.txt'"; - private static final String case4 = "'file:///tests/test.txt'"; - private static final String case5 = "'tests/test.txt'"; - - public static final List cases = Arrays.asList(case1, case2, case3, case4, case5); - - private static final Map> METHOD_ARGUMENTS = Map.of( - "query", cases.stream().map( - filePath -> "\"RETURN 1\", " + filePath + ", {}" - ).toList(), - "all", cases.stream().map( - filePath -> filePath + ", {}" - ).toList(), - "data", cases.stream().map( - filePath -> "[], [], " + filePath + ", {}" - ).toList(), - "graph", cases.stream().map( - filePath -> "{nodes: [], relationships: []}, " + filePath + ", {}" - ).toList() - ); - - @Parameterized.Parameters - public static Collection data() { - return ExportCoreSecurityTest.data(METHOD_ARGUMENTS); + private File getDir() { + if (subDirCases.contains(fileName)) { + return subDirectory; + } + return directory; } - @Test - public void testPathTraversal() { - assertPathTraversalWithoutErrors(db, apocProcedure, subDirectory, - (r) -> assertTrue(((String) r.get("file")).contains("tests/test.txt"))); + private void assertPathTraversalWithoutErrors(File directory) { + File file = new File(directory.getAbsolutePath(), FILENAME); + SecurityTestUtil.assertPathTraversalWithoutErrors(db, apocProcedure, fileName, file); } + + // tests with `apoc.import.file.use_neo4j_config=false` not implemented because the results can be different + // e.g. based on project folder name, so the exported file can be basically everywhere + } + + public static class TestCypherSchema { - private final String apocProcedure = "apoc.export.cypher.schema(%s)"; + private final String apocProcedure = "CALL apoc.export.cypher.schema(%s)"; @Test public void testIllegalFSAccessExportCypherSchema() { + setFileExport(false); QueryExecutionException e = Assert.assertThrows(QueryExecutionException.class, - () -> TestUtil.testCall(db, String.format("CALL " + apocProcedure, "'./hello', {}"), (r) -> {}) + () -> TestUtil.testCall(db, String.format(apocProcedure, "'./hello', {}"), (r) -> { + }) ); assertError(e, ApocConfig.EXPORT_TO_FILE_ERROR, RuntimeException.class, apocProcedure); } @Test public void testIllegalExternalFSAccessExportCypherSchema() { - assertPathTraversalError(db, String.format("CALL " + apocProcedure, "'../hello', {}"), - e -> assertError(e, FileUtils.ACCESS_OUTSIDE_DIR_ERROR, IOException.class, apocProcedure)); + setFileExport(true); + assertPathTraversalError(db, String.format(apocProcedure, "'../hello', {}"), Map.of(), + e -> assertError((Exception) e.get(ERROR_KEY), FileUtils.ACCESS_OUTSIDE_DIR_ERROR, IOException.class, (String) e.get(PROCEDURE_KEY))); } } - - public static String getApocProcedure(String exportMethod, String exportMethodType, String exportMethodArguments) { - return "apoc.export." + exportMethod + "." + exportMethodType + "(" + exportMethodArguments + ")"; - } - - public static void assertPathTraversalError(GraphDatabaseService db, String query, Consumer exceptionConsumer) { - setFileExport(true); - - QueryExecutionException e = Assert.assertThrows(QueryExecutionException.class, - () -> TestUtil.testCall(db, query, (r) -> {}) - ); - - exceptionConsumer.accept(e); - - setFileExport(false); - } - - public static void assertPathTraversalWithoutErrors(GraphDatabaseService db, String apocProcedure, File directory, Consumer> consumer) { - setFileExport(true); - - TestUtil.testCall(db, "CALL " + apocProcedure, consumer); - - File f = new File(directory.getAbsolutePath() + "/test.txt"); - TestCase.assertTrue(f.exists()); - TestCase.assertTrue(f.delete()); - } - - public static void assertPathTraversalError(GraphDatabaseService db, String apocProcedure) { - assertPathTraversalError(db, apocProcedure, - e -> assertError(e, FileUtils.ACCESS_OUTSIDE_DIR_ERROR, IOException.class, apocProcedure) - ); - } - } \ No newline at end of file diff --git a/core/src/test/java/apoc/export/ImportAndLoadCoreSecurityTest.java b/core/src/test/java/apoc/export/ImportAndLoadCoreSecurityTest.java new file mode 100644 index 000000000..696341c71 --- /dev/null +++ b/core/src/test/java/apoc/export/ImportAndLoadCoreSecurityTest.java @@ -0,0 +1,321 @@ +package apoc.export; + +import apoc.ApocConfig; +import apoc.export.csv.ImportCsv; +import apoc.export.graphml.ExportGraphML; +import apoc.export.json.ImportJson; +import apoc.load.LoadArrow; +import apoc.load.LoadJson; +import apoc.load.Xml; +import apoc.util.SensitivePathGenerator; +import apoc.util.TestUtil; +import com.nimbusds.jose.util.Pair; +import inet.ipaddr.IPAddressString; +import org.apache.commons.lang3.exception.ExceptionUtils; +import org.assertj.core.api.Assertions; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.neo4j.configuration.GraphDatabaseInternalSettings; +import org.neo4j.configuration.GraphDatabaseSettings; +import org.neo4j.graphdb.QueryExecutionException; +import org.neo4j.graphdb.Result; +import org.neo4j.test.rule.DbmsRule; +import org.neo4j.test.rule.ImpermanentDbmsRule; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import static apoc.export.SecurityTestUtil.ALLOWED_EXCEPTIONS; +import static apoc.export.SecurityTestUtil.IMPORT_PROCEDURES; +import static apoc.export.SecurityTestUtil.LOAD_PROCEDURES; +import static apoc.export.SecurityTestUtil.setImportFileApocConfigs; +import static apoc.util.FileTestUtil.createTempFolder; +import static apoc.util.FileUtils.ACCESS_OUTSIDE_DIR_ERROR; +import static apoc.util.FileUtils.ERROR_READ_FROM_FS_NOT_ALLOWED; +import static apoc.util.TestUtil.testCall; +import static junit.framework.TestCase.assertTrue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.fail; + +@RunWith(Parameterized.class) +public class ImportAndLoadCoreSecurityTest { + private static final Path TEMP_FOLDER = createTempFolder(); + + // base path: "../../../../etc/passwd" + private static final String ABSOLUTE_URL = SensitivePathGenerator.etcPasswd().getLeft(); + private static final String ABSOLUTE_URL_WITH_FILE_PREFIX = "file:" + ABSOLUTE_URL; + private static final String ABSOLUTE_URL_WITH_FILE_SLASH = "file:/" + ABSOLUTE_URL; + private static final String ABSOLUTE_URL_WITH_FILE_DOUBLE_SLASH = "file://" + ABSOLUTE_URL; + private static final String ABSOLUTE_URL_WITH_FILE_TRIPLE_SLASH = "file:///" + ABSOLUTE_URL; + + // base path: "/etc/passwd" + private static final String RELATIVE_URL = SensitivePathGenerator.etcPasswd().getRight(); + private static final String RELATIVE_URL_WITH_FILE_SLASH = "file:" + RELATIVE_URL; + private static final String RELATIVE_URL_WITH_FILE_DOUBLE_SLASH = "file:/" + RELATIVE_URL; + private static final String RELATIVE_URL_WITH_FILE_TRIPLE_SLASH = "file://" + RELATIVE_URL; + + + private final String apocProcedure; + private final String importMethod; + private final String fileName; + + public ImportAndLoadCoreSecurityTest(String method, String methodArguments, String fileName) { + this.apocProcedure = "CALL " + method + methodArguments; + this.importMethod = method; + this.fileName = fileName; + } + + @Parameterized.Parameters(name = "Procedure: {0}{1}, fileName: {2}") + public static Collection data() { + // transform `Pair(, )` to `Pair(apoc.import. , )` + List> importAndLoadProcedures = IMPORT_PROCEDURES + .map(e -> Pair.of( "apoc.import." + e.getLeft() , e.getRight() )) + .collect(Collectors.toList()); + + // transform `Pair(, )` to `Pair(apoc.load. , )` + List> loadProcedures = LOAD_PROCEDURES + .map(e -> Pair.of( "apoc.load." + e.getLeft() , e.getRight() )) + .toList(); + + importAndLoadProcedures.addAll(loadProcedures); + + return getParameterData(importAndLoadProcedures); + } + + private static List getParameterData(List> importAndLoadProcedures) { + // from a stream of fileNames and a List of Pair + // returns a List of String[]{ procName, procArgs, fileName } + + Stream fileNames = Stream.of(ABSOLUTE_URL, + ABSOLUTE_URL_WITH_FILE_PREFIX, + ABSOLUTE_URL_WITH_FILE_SLASH, + ABSOLUTE_URL_WITH_FILE_DOUBLE_SLASH, + ABSOLUTE_URL_WITH_FILE_TRIPLE_SLASH, + RELATIVE_URL, + RELATIVE_URL_WITH_FILE_SLASH, + RELATIVE_URL_WITH_FILE_DOUBLE_SLASH, + RELATIVE_URL_WITH_FILE_TRIPLE_SLASH); + + return fileNames.flatMap(fileName -> importAndLoadProcedures + .stream() + .map( + procPair -> new String[] { procPair.getLeft(), procPair.getRight(), fileName } + ) + ).toList(); + } + + @ClassRule + public static DbmsRule db = new ImpermanentDbmsRule() + .withSetting(GraphDatabaseSettings.load_csv_file_url_root, TEMP_FOLDER) + .withSetting(GraphDatabaseInternalSettings.cypher_ip_blocklist, List.of(new IPAddressString("127.168.0.0/8"))); + + + @BeforeClass + public static void setUp() throws IOException { + TestUtil.registerProcedure(db, + // import procedures (ExportGraphML contains the `apoc.import.graphml` too) + ImportJson.class, Xml.class, ImportCsv.class, ExportGraphML.class, + // load procedures (Xml contains both `apoc.load.xml` and `apoc.import.xml` procedures) + LoadJson.class, LoadArrow.class); + } + + + @Test + public void testIllegalFSAccessWithDifferentApocConfs() { + // apoc.import.file.enabled=true + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=true + setImportFileApocConfigs(true, true, true); + assertIpAddressBlocked(); + + // apoc.import.file.enabled=true + // apoc.import.file.use_neo4j_config=false + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(true, false, false); + assertIpAddressBlocked(); + + // apoc.import.file.enabled=true + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(true, true, false); + assertIpAddressBlocked(); + + // apoc.import.file.enabled=true + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(true, false, true); + assertIpAddressBlocked(); + + // apoc.import.file.enabled=false + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(false, true, false); + assertIpAddressBlocked(); + + // apoc.import.file.enabled=false + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=true + setImportFileApocConfigs(false, true, true); + assertIpAddressBlocked(); + + // apoc.import.file.enabled=false + // apoc.import.file.use_neo4j_config=false + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(false, false, true); + assertIpAddressBlocked(); + + // apoc.import.file.enabled=false + // apoc.import.file.use_neo4j_config=false + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(false, false, false); + assertIpAddressBlocked(); + } + + @Test + public void testImportFileDisabled() { + // all assertions with `apoc.import.file.enabled=false` + + // apoc.import.file.use_neo4j_config=false + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(false, false, false); + assertImportDisabled(); + + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(false, true, false); + assertImportDisabled(); + + // apoc.import.file.use_neo4j_config=false + // apoc.import.file.allow_read_from_filesystem=true + setImportFileApocConfigs(false, false, true); + assertImportDisabled(); + + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=true + setImportFileApocConfigs(false, true, true); + assertImportDisabled(); + } + + @Test + public void testIllegalFSAccessWithImportAndUseNeo4jConfsEnabled() { + // apoc.import.file.enabled=true + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(true, true, false); + assertReadFromFsNotAllowed(); + } + + @Test + public void testImportOutsideDirNotAllowedWithAllApocFileConfigsEnabled() { + // apoc.import.file.enabled=true + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=true + setImportFileApocConfigs(true, true, true); + + // only `../../../etc/passwd` throw the error, other urls just don't find the file, + // i.e.: `file:/../../../etc/passwd`, `file://../../../etc/passwd` and `file:///../../../etc/passwd` + // and relative ones (like `/etc/passwd`) + if (fileName.equals(ABSOLUTE_URL) || fileName.equals(ABSOLUTE_URL_WITH_FILE_PREFIX)) { + assertImportOutsideDirNotAllowed(); + } else { + assertFileNotExists(); + } + } + + @Test + public void testReadSensitiveFileWorksWithApocUseNeo4jConfigDisabled() { + // all checks with `apoc.import.file.use_neo4j_config=false` + + // apoc.import.file.enabled=true + // apoc.import.file.allow_read_from_filesystem=true + setImportFileApocConfigs(true, false, true); + shouldRead(); + + // apoc.import.file.enabled=false + // apoc.import.file.allow_read_from_filesystem=false + setImportFileApocConfigs(true, false, false); + shouldRead(); + } + + private void assertIpAddressBlocked() { + Stream.of("https", "http", "ftp").forEach(protocol -> { + + String url = String.format("%s://127.168.0.0/test.file", protocol); + QueryExecutionException e = assertThrows(QueryExecutionException.class, + () -> testCall(db, + apocProcedure, + Map.of("fileName", url), + (r) -> {} + ) + ); + assertTrue(e.getMessage().contains("access to /127.168.0.0 is blocked via the configuration property internal.dbms.cypher_ip_blocklist")); + }); + } + + private void assertImportDisabled() { + assertFailingProcedure(ApocConfig.LOAD_FROM_FILE_ERROR, RuntimeException.class); + } + + private void assertReadFromFsNotAllowed() { + assertFailingProcedure(String.format(ERROR_READ_FROM_FS_NOT_ALLOWED, fileName), RuntimeException.class); + } + + private void assertFailingProcedure(String expectedError, Class exceptionClass) { + final String message = apocProcedure + " should throw an exception"; + + try { + db.executeTransactionally(apocProcedure, + Map.of("fileName", fileName), + Result::resultAsString); + fail(message); + } catch (Exception e) { + TestUtil.assertError(e, expectedError, exceptionClass, apocProcedure); + } + } + + private void shouldRead() { + // the `file://../../../etc/passwd` is not found unlike the other similar urls, + // i.e.: `file:/../../../etc/passwd`, `file:///../../../etc/passwd` and `../../../etc/passwd` + if (fileName.equals(RELATIVE_URL_WITH_FILE_DOUBLE_SLASH)) { + assertFileNotExists(); + return; + } + try { + db.executeTransactionally(apocProcedure, + Map.of("fileName", fileName), + Result::resultAsString); + } catch (Exception e) { + if (ALLOWED_EXCEPTIONS.containsKey(importMethod)) { + assertEquals(ALLOWED_EXCEPTIONS.get(importMethod), ExceptionUtils.getRootCause(e).getClass()); + } + } + } + + private void assertImportOutsideDirNotAllowed() { + assertFailingProcedure(ACCESS_OUTSIDE_DIR_ERROR, IOException.class); + } + + private void assertFileNotExists() { + try { + db.executeTransactionally(apocProcedure, + Map.of("fileName", fileName), + Result::resultAsString); + } catch (Exception e) { + final Throwable rootCause = ExceptionUtils.getRootCause(e); + assertTrue(rootCause instanceof IOException); + String message = e.getMessage(); + Assertions.assertThat(message).contains("Cannot open file "); + Assertions.assertThat(message).contains(" for reading."); + } + } + +} diff --git a/core/src/test/java/apoc/export/SecurityTestUtil.java b/core/src/test/java/apoc/export/SecurityTestUtil.java new file mode 100644 index 000000000..8d6a74368 --- /dev/null +++ b/core/src/test/java/apoc/export/SecurityTestUtil.java @@ -0,0 +1,144 @@ +package apoc.export; + +import apoc.ApocConfig; +import apoc.util.TestUtil; +import apoc.util.Util; +import com.ctc.wstx.exc.WstxUnexpectedCharException; +import com.fasterxml.jackson.core.JsonParseException; +import com.nimbusds.jose.util.Pair; +import junit.framework.TestCase; +import org.junit.Assert; +import org.neo4j.graphdb.GraphDatabaseService; +import org.neo4j.graphdb.QueryExecutionException; +import org.xml.sax.SAXParseException; + +import java.io.File; +import java.util.List; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.function.Consumer; +import java.util.stream.Stream; + +import static apoc.ApocConfig.APOC_EXPORT_FILE_ENABLED; +import static apoc.ApocConfig.APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM; +import static apoc.ApocConfig.APOC_IMPORT_FILE_ENABLED; +import static apoc.ApocConfig.APOC_IMPORT_FILE_USE_NEO4J_CONFIG; +import static apoc.ApocConfig.apocConfig; +import static apoc.util.TestUtil.assertError; + +public class SecurityTestUtil { + public static final String PROCEDURE_KEY = "procedure"; + public static final String ERROR_KEY = "error"; + + public static final Map> ALLOWED_EXCEPTIONS = Map.of( + // load allowed exception + "apoc.load.json", JsonParseException.class, + "apoc.load.jsonArray", JsonParseException.class, + "apoc.load.jsonParams", JsonParseException.class, + "apoc.load.xml", SAXParseException.class, + + // import allowed exception + "apoc.import.json", JsonParseException.class, + "apoc.import.csv", NoSuchElementException.class, + "apoc.import.graphml", JsonParseException.class, + "apoc.import.xml", WstxUnexpectedCharException.class + ); + + public static List> EXPORT_PROCEDURES = List.of( + Pair.of("query", "'RETURN 1', $fileName, {}"), + Pair.of("all", "$fileName, {}"), + Pair.of("data", "[], [], $fileName, {}"), + Pair.of("graph", "{nodes: [], relationships: []}, $fileName, {}") + ); + + public static Stream> IMPORT_PROCEDURES = Stream.of( + Pair.of("json", "($fileName)"), + Pair.of("csv", "([{fileName: $fileName, labels: ['Person']}], [], {})"), + Pair.of("csv", "([], [{fileName: $fileName, type: 'KNOWS'}], {})"), + Pair.of("graphml", "($fileName, {})"), + Pair.of("xml", "($fileName)") + ); + + public static Stream> LOAD_PROCEDURES = Stream.of( + Pair.of("json", "($fileName, '', {})"), + Pair.of("jsonArray", "($fileName, '', {})"), + Pair.of("jsonParams", "($fileName, {}, '')"), + Pair.of("xml", "($fileName, '', {}, false)"), + Pair.of("arrow", "($fileName)") + ); + + + public static void assertPathTraversalError(GraphDatabaseService db, String query, Map params, Consumer exceptionConsumer) { + + QueryExecutionException e = Assert.assertThrows(QueryExecutionException.class, + () -> TestUtil.testCall(db, query, params, (r) -> {}) + ); + + // this consumer accept a Map.of("error", , "procedure", ) + exceptionConsumer.accept( + Util.map(ERROR_KEY, e, PROCEDURE_KEY, query) + ); + } + + public static String getApocProcedure(String exportMethod, String exportMethodType, String exportMethodArguments) { + return "CALL apoc.export." + exportMethod + "." + exportMethodType + "(" + exportMethodArguments + ")"; + } + + public static void testsWithExportDisabled(GraphDatabaseService db, String apocProcedure, String fileName) { + testsWithExportDisabled(db, apocProcedure, fileName, ApocConfig.EXPORT_TO_FILE_ERROR); + } + + public static void testsWithExportDisabled(GraphDatabaseService db, String apocProcedure, String fileName, String error) { + // all assertions with `apoc.export.file.enabled=true` + + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=true + setExportFileApocConfigs(false, true, true); + assertExportDisabled(db, apocProcedure, fileName, error); + + // apoc.import.file.use_neo4j_config=true + // apoc.import.file.allow_read_from_filesystem=false + setExportFileApocConfigs(false, true, false); + assertExportDisabled(db, apocProcedure, fileName, error); + + // apoc.import.file.use_neo4j_config=false + // apoc.import.file.allow_read_from_filesystem=true + setExportFileApocConfigs(false, false, true); + assertExportDisabled(db, apocProcedure, fileName, error); + + // apoc.import.file.use_neo4j_config=false + // apoc.import.file.allow_read_from_filesystem=false + setExportFileApocConfigs(false, false, false); + assertExportDisabled(db, apocProcedure, fileName, error); + } + + private static void assertExportDisabled(GraphDatabaseService db, String apocProcedure, String fileName, String error) { + QueryExecutionException e = Assert.assertThrows(QueryExecutionException.class, + () -> TestUtil.testCall(db, apocProcedure, Map.of("fileName", fileName), (r) -> {}) + ); + + assertError(e, error, RuntimeException.class, apocProcedure); + } + + public static void assertPathTraversalWithoutErrors(GraphDatabaseService db, String apocProcedure, String fileName, File file) { + TestUtil.testCall(db, apocProcedure, Map.of("fileName", fileName), r -> {}); + + TestCase.assertTrue("The file doesn't exists", file.exists()); + TestCase.assertTrue(file.delete()); + } + + public static void setImportFileApocConfigs(boolean importEnabled, boolean useNeo4jConfs, boolean allowReadFromFs) { + apocConfig().setProperty(APOC_IMPORT_FILE_ENABLED, importEnabled); + setFileConfigsCommon(useNeo4jConfs, allowReadFromFs); + } + + public static void setExportFileApocConfigs(boolean exportEnabled, boolean useNeo4jConfs, boolean allowReadFromFs) { + apocConfig().setProperty(APOC_EXPORT_FILE_ENABLED, exportEnabled); + setFileConfigsCommon(useNeo4jConfs, allowReadFromFs); + } + + private static void setFileConfigsCommon(boolean useNeo4jConfs, boolean allowReadFromFs) { + apocConfig().setProperty(APOC_IMPORT_FILE_USE_NEO4J_CONFIG, useNeo4jConfs); + apocConfig().setProperty(APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM, allowReadFromFs); + } +} diff --git a/core/src/test/java/apoc/export/arrow/ExportArrowSecurityTest.java b/core/src/test/java/apoc/export/arrow/ExportArrowSecurityTest.java index 3cebe2a1b..ffab329aa 100644 --- a/core/src/test/java/apoc/export/arrow/ExportArrowSecurityTest.java +++ b/core/src/test/java/apoc/export/arrow/ExportArrowSecurityTest.java @@ -19,11 +19,10 @@ package apoc.export.arrow; import apoc.export.ExportCoreSecurityTest; +import apoc.export.SecurityTestUtil; import apoc.meta.Meta; -import apoc.util.FileUtils; import apoc.util.TestUtil; -import junit.framework.TestCase; -import org.junit.Assert; +import com.nimbusds.jose.util.Pair; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Test; @@ -31,24 +30,29 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.neo4j.configuration.GraphDatabaseSettings; -import org.neo4j.graphdb.QueryExecutionException; -import org.neo4j.graphdb.Result; import org.neo4j.test.rule.DbmsRule; import org.neo4j.test.rule.ImpermanentDbmsRule; import java.io.File; -import java.io.IOException; import java.util.Collection; import java.util.List; import java.util.Map; - -import static apoc.export.ExportCoreSecurityTest.assertPathTraversalError; -import static apoc.export.ExportCoreSecurityTest.assertPathTraversalWithoutErrors; -import static apoc.export.ExportCoreSecurityTest.getApocProcedure; -import static apoc.export.ExportCoreSecurityTest.setFileExport; -import static apoc.util.TestUtil.assertError; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; +import java.util.function.Consumer; +import java.util.stream.Collectors; + +import static apoc.export.ExportCoreSecurityTest.FILENAME; +import static apoc.export.ExportCoreSecurityTest.PARAM_NAMES; +import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.case26; +import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.casesAllowed; +import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.EXCEPTION_NOT_FOUND_CONSUMER; +import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.dataPairs; +import static apoc.export.ExportCoreSecurityTest.TestPathTraversalIsNormalisedWithinDirectory.MAIN_DIR_CONSUMER; +import static apoc.export.ExportCoreSecurityTest.TestPathTraversalIsNormalisedWithinDirectory.SUB_DIR_CONSUMER; +import static apoc.export.ExportCoreSecurityTest.TestPathTraversalIsNormalisedWithinDirectory.mainDirCases; +import static apoc.export.ExportCoreSecurityTest.TestPathTraversalIsNormalisedWithinDirectory.subDirCases; +import static apoc.export.SecurityTestUtil.getApocProcedure; +import static apoc.export.SecurityTestUtil.setExportFileApocConfigs; +import static apoc.export.arrow.ExportArrowService.EXPORT_TO_FILE_ARROW_ERROR; @RunWith(Enclosed.class) public class ExportArrowSecurityTest { @@ -66,19 +70,11 @@ public class ExportArrowSecurityTest { directoryWithSamePrefix.mkdirs(); } - private static Map> getArguments(List cases) { - return Map.of( - "query", cases.stream().map( - filePath -> filePath + ", \"RETURN 1\", {}" - ).toList(), - "all", cases.stream().map( - filePath -> filePath + ", {}" - ).toList(), - "graph", cases.stream().map( - filePath -> filePath + ", {nodes: [], relationships: []}, {}" - ).toList() - ); - } + public static List> EXPORT_PROCEDURES = List.of( + Pair.of("query", "$fileName, 'RETURN 1', {}"), + Pair.of("all", "$fileName, {}"), + Pair.of("graph", "$fileName, {nodes: [], relationships: []}, {}") + ); @ClassRule public static DbmsRule db = new ImpermanentDbmsRule() @@ -87,238 +83,150 @@ private static Map> getArguments(List cases) { @BeforeClass public static void setUp() { TestUtil.registerProcedure(db, ExportArrow.class, Meta.class); - setFileExport(false); - } - - private static Collection data(Map> apocProcedureArguments) { - return ExportCoreSecurityTest.data(apocProcedureArguments, APOC_EXPORT_PROCEDURE_NAME); } - @RunWith(Parameterized.class) - public static class TestIllegalFSAccess { - private final String apocProcedure; - - public TestIllegalFSAccess(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); - } - - private static final Map> APOC_PROCEDURE_ARGUMENTS = Map.of( - "query", List.of( - "'./hello', \"RETURN 'hello' as key\", {}", - "'./hello', \"RETURN 'hello' as key\", {stream:true}", - "' ', \"RETURN 'hello' as key\", {}" - ), - "all", List.of( - "'./hello', {}", - "'./hello', {stream:true}", - "' ', {}" - ), - "graph", List.of( - "'./hello', {nodes: [], relationships: []}, {}", - "'./hello', {nodes: [], relationships: []}, {stream:true}", - "' ', {nodes: [], relationships: []}, {}" - ) - ); - - @Parameterized.Parameters - public static Collection data() { - return ExportArrowSecurityTest.data(APOC_PROCEDURE_ARGUMENTS); - } - - @Test - public void testIllegalFSAccessExport() { - QueryExecutionException e = Assert.assertThrows(QueryExecutionException.class, - () -> TestUtil.testCall(db, "CALL " + apocProcedure, (r) -> {}) - ); - - assertError(e, ExportArrowService.EXPORT_TO_FILE_ARROW_ERROR, RuntimeException.class, apocProcedure); - } + private static Collection getParameterData(List>> fileAndErrors) { + return ExportCoreSecurityTest.getParameterData(fileAndErrors, EXPORT_PROCEDURES, APOC_EXPORT_PROCEDURE_NAME); } + /** + * These test with `apoc.import.file.use_neo4j_config=true` + * should fail because they access a directory which isn't the configured directory + */ @RunWith(Parameterized.class) public static class TestIllegalExternalFSAccess { private final String apocProcedure; + private final String fileName; + private final Consumer consumer; - public TestIllegalExternalFSAccess(String exportMethod, String exportMethodType, String exportMethodArguments) { + public TestIllegalExternalFSAccess(String exportMethod, String exportMethodType, String exportMethodArguments, + String fileName, + Consumer consumer) { this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); + this.fileName = fileName; + this.consumer = consumer; } - private static final Map> METHOD_ARGUMENTS = Map.of( - "query", List.of( - "'../hello', \"RETURN 'hello' as key\", {}", - "'file:../hello', \"RETURN 'hello' as key\", {}" - ), - "all", List.of( - "'../hello', {}", - "'file:../hello', {}" - ), - "graph", List.of( - "'../hello', {nodes: [], relationships: []}, {}", - "'file:../hello', {nodes: [], relationships: []}, {}" - ) - ); - - @Parameterized.Parameters - public static Collection data() { - return ExportArrowSecurityTest.data(METHOD_ARGUMENTS); + @Parameterized.Parameters(name = PARAM_NAMES) + public static Collection data() { + return ExportArrowSecurityTest.getParameterData(dataPairs); } @Test - public void testIllegalExternalFSAccessExport() { - final String message = apocProcedure + " should throw an exception"; - try { - setFileExport(true); - db.executeTransactionally("CALL " + apocProcedure, Map.of(), - Result::resultAsString); - fail(message); - } catch (Exception e) { - assertError(e, FileUtils.ACCESS_OUTSIDE_DIR_ERROR, IOException.class, apocProcedure); - } finally { - setFileExport(false); - } + public void testsWithExportDisabled() { + SecurityTestUtil.testsWithExportDisabled(db, apocProcedure, fileName, EXPORT_TO_FILE_ARROW_ERROR); } - } - @RunWith(Parameterized.class) - public static class TestPathTraversalAccess { - private final String apocProcedure; - - public TestPathTraversalAccess(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); - } + @Test + public void testIllegalExternalFSAccessExportWithExportAndUseNeo4jConfEnabled() { + // all assertions with `apoc.export.file.enabled=true` and `apoc.import.file.use_neo4j_config=true` - private static final Map> METHOD_ARGUMENTS = getArguments( - ExportCoreSecurityTest.TestPathTraversalAccess.cases - ); + // apoc.import.file.allow_read_from_filesystem=false + setExportFileApocConfigs(true, true, false); + SecurityTestUtil.assertPathTraversalError(db, apocProcedure, Map.of("fileName", fileName), consumer); - @Parameterized.Parameters - public static Collection data() { - return ExportArrowSecurityTest.data(METHOD_ARGUMENTS); + // apoc.import.file.allow_read_from_filesystem=true + setExportFileApocConfigs(true, true, true); + SecurityTestUtil.assertPathTraversalError(db, apocProcedure, Map.of("fileName", fileName), consumer); } @Test - public void testPathTraversal() { - assertPathTraversalError(db, "CALL " + apocProcedure); - } - } + public void testWithUseNeo4jConfDisabled() { + // all assertions with `apoc.export.file.enabled=true` and `apoc.import.file.use_neo4j_config=false` - /** - * All of these will resolve to a local path after normalization which will point to - * a non-existing directory in our import folder: /apoc. Causing them to error that is - * not found. They all attempt to exit the import folder back to the apoc folder: - * Directory Layout: .../apoc/core/target/import - */ - @RunWith(Parameterized.class) - public static class TestPathTraversalIsNormalised { - private final String apocProcedure; + // apoc.import.file.allow_read_from_filesystem=true + setExportFileApocConfigs(true, false, true); + testWithUseNeo4jConfFalse(); - public TestPathTraversalIsNormalised(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); + // apoc.import.file.allow_read_from_filesystem=false + setExportFileApocConfigs(true, false, false); + testWithUseNeo4jConfFalse(); } - private static final Map> METHOD_ARGUMENTS = getArguments( - ExportCoreSecurityTest.TestPathTraversalIsNormalised.cases - ); + private void testWithUseNeo4jConfFalse() { + // with `apoc.import.file.use_neo4j_config=false` this file export could outside the project + if (fileName.equals(case26)) { + return; + } - @Parameterized.Parameters - public static Collection data() { - return ExportArrowSecurityTest.data(METHOD_ARGUMENTS); + if (casesAllowed.contains(fileName)) { + assertPathTraversalWithoutErrors(); + } else { + SecurityTestUtil.assertPathTraversalError(db, apocProcedure, Map.of("fileName", fileName), EXCEPTION_NOT_FOUND_CONSUMER); + } } - @Test - public void testPathTraversal() { - assertPathTraversalError(db, "CALL " + apocProcedure, - e -> TestCase.assertTrue(e.getMessage().contains("apoc/test.txt (No such file or directory)")) - ); + private void assertPathTraversalWithoutErrors() { + SecurityTestUtil.assertPathTraversalWithoutErrors(db, apocProcedure, fileName, new File("../", FILENAME)); } } /** - * These tests normalize the path to be within the import directory and make the file there. + * These tests normalize the path to be within the import directory (or subdirectory) and make the file there. * Some attempt to exit the directory. - * They result in a file name test.txt being created (and deleted after). */ @RunWith(Parameterized.class) public static class TestPathTraversalIsNormalisedWithinDirectory { + private final String apocProcedure; + private final String fileName; + private final Consumer consumer; - public TestPathTraversalIsNormalisedWithinDirectory(String exportMethod, String exportMethodType, String exportMethodArguments) { + public TestPathTraversalIsNormalisedWithinDirectory(String exportMethod, String exportMethodType, String exportMethodArguments, + String fileName, + Consumer consumer) { this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); + this.fileName = fileName; + this.consumer = consumer; } - public static final List cases = ExportCoreSecurityTest.TestPathTraversalIsNormalisedWithinDirectory.cases; - private static final Map> METHOD_ARGUMENTS = getArguments(cases); + @Parameterized.Parameters(name = PARAM_NAMES) + public static Collection data() { + List>> collect = mainDirCases.stream().map(i -> Pair.of(i, MAIN_DIR_CONSUMER)).collect(Collectors.toList()); + List>> collect2 = subDirCases.stream().map(i -> Pair.of(i, SUB_DIR_CONSUMER)).toList(); + collect.addAll(collect2); - @Parameterized.Parameters - public static Collection data() { - return ExportArrowSecurityTest.data(METHOD_ARGUMENTS); + return ExportArrowSecurityTest.getParameterData(collect); } @Test public void testPathTraversal() { - assertPathTraversalWithoutErrors(db, apocProcedure, directory, - (r) -> assertTrue(((String) r.get("file")).contains("test.txt"))); - } - } - - /* - * These test cases attempt to access a directory with the same prefix as the import directory. This is design to - * test "directoryName.startsWith" logic which is a common path traversal bug. - \* - * All these tests should fail because they access a directory which isn't the configured directory - */ - @RunWith(Parameterized.class) - public static class TestPathTraversalIsWithSimilarDirectoryName { - private final String apocProcedure; + setExportFileApocConfigs(true, true, false); + File dir = getDir(); - public TestPathTraversalIsWithSimilarDirectoryName(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); - } - - private static final Map> METHOD_ARGUMENTS = getArguments( - ExportCoreSecurityTest.TestPathTraversalIsWithSimilarDirectoryName.cases - ); - - @Parameterized.Parameters - public static Collection data() { - return ExportArrowSecurityTest.data(METHOD_ARGUMENTS); + assertPathTraversalWithoutErrors(dir); } @Test - public void testPathTraversal() { - assertPathTraversalError(db, "CALL " + apocProcedure); + public void testIllegalFSAccessExport() { + SecurityTestUtil.testsWithExportDisabled(db, apocProcedure, fileName, EXPORT_TO_FILE_ARROW_ERROR); } - } + @Test + public void testWithUseNeo4jConfDisabled() { + // all assertions with `apoc.export.file.enabled=true` and `apoc.import.file.use_neo4j_config=true` + File dir = getDir(); - /** - * These tests normalize the path to be within the import directory and step into a subdirectory - * to make the file there. - * Some attempt to exit the directory. - * They result in a file name test.txt in the directory /tests being created (and deleted after). - */ - @RunWith(Parameterized.class) - public static class TestPathTraversAllowedWithinDirectory { - private final String apocProcedure; + // apoc.import.file.allow_read_from_filesystem=false + setExportFileApocConfigs(true, true, false); + assertPathTraversalWithoutErrors(dir); - public TestPathTraversAllowedWithinDirectory(String exportMethod, String exportMethodType, String exportMethodArguments) { - this.apocProcedure = getApocProcedure(exportMethod, exportMethodType, exportMethodArguments); + // apoc.import.file.allow_read_from_filesystem=true + setExportFileApocConfigs(true, true, true); + assertPathTraversalWithoutErrors(dir); } - private static final Map> METHOD_ARGUMENTS = getArguments( - ExportCoreSecurityTest.TestPathTraversAllowedWithinDirectory.cases - ); - - @Parameterized.Parameters - public static Collection data() { - return ExportArrowSecurityTest.data(METHOD_ARGUMENTS); + private File getDir() { + if (subDirCases.contains(fileName)) { + return subDirectory; + } + return directory; } - @Test - public void testPathTraversal() { - assertPathTraversalWithoutErrors(db, apocProcedure, subDirectory, - (r) -> assertTrue(((String) r.get("file")).contains("tests/test.txt"))); + private void assertPathTraversalWithoutErrors(File directory) { + File file = new File(directory.getAbsolutePath(), FILENAME); + SecurityTestUtil.assertPathTraversalWithoutErrors(db, apocProcedure, fileName, file); } } } \ No newline at end of file diff --git a/core/src/test/java/apoc/load/LoadCoreSecurityTest.java b/core/src/test/java/apoc/load/LoadCoreSecurityTest.java index f242d0379..e69de29bb 100644 --- a/core/src/test/java/apoc/load/LoadCoreSecurityTest.java +++ b/core/src/test/java/apoc/load/LoadCoreSecurityTest.java @@ -1,164 +0,0 @@ -/* - * Copyright (c) "Neo4j" - * Neo4j Sweden AB [http://neo4j.com] - * - * This file is part of Neo4j. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package apoc.load; - -import apoc.ApocConfig; -import apoc.util.FileUtils; -import apoc.util.SensitivePathGenerator; -import apoc.util.TestUtil; -import com.fasterxml.jackson.core.JsonParseException; -import org.apache.commons.lang3.exception.ExceptionUtils; -import org.junit.BeforeClass; -import org.junit.ClassRule; -import org.junit.Test; -import org.junit.experimental.runners.Enclosed; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.neo4j.configuration.GraphDatabaseSettings; -import org.neo4j.graphdb.Result; -import org.neo4j.test.rule.DbmsRule; -import org.neo4j.test.rule.ImpermanentDbmsRule; -import org.xml.sax.SAXParseException; - -import java.io.File; -import java.nio.file.Path; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.UUID; -import java.util.stream.Collectors; - -import static apoc.ApocConfig.APOC_IMPORT_FILE_ENABLED; -import static apoc.ApocConfig.apocConfig; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; - -@RunWith(Enclosed.class) -public class LoadCoreSecurityTest { - - public static Path import_folder; - - static { - try { - import_folder = File.createTempFile(UUID.randomUUID().toString(), "tmp") - .getParentFile().toPath(); - } catch (Exception e) { - throw new RuntimeException(e); - } - } - - @ClassRule - public static DbmsRule db = new ImpermanentDbmsRule() - .withSetting(GraphDatabaseSettings.load_csv_file_url_root, import_folder); - - @BeforeClass - public static void setUp() { - TestUtil.registerProcedure(db, LoadJson.class, Xml.class); - apocConfig().setProperty(APOC_IMPORT_FILE_ENABLED, false); - } - - private static final Map> APOC_PROCEDURE_WITH_ARGUMENTS = Map.of( - "json", List.of("($fileName, '', {})"), - "jsonArray", List.of("($fileName, '', {})"), - "jsonParams", List.of("($fileName, {}, '')"), - "xml", List.of("($fileName, '', {}, false)")); - - private static final Map> ALLOWED_EXCEPTIONS = Map.of( - "json", JsonParseException.class, - "jsonArray", JsonParseException.class, - "jsonParams", JsonParseException.class, - "xml", SAXParseException.class); - - - private static Collection data() { - return APOC_PROCEDURE_WITH_ARGUMENTS.entrySet() - .stream() - .flatMap(e -> e.getValue().stream().map(arg -> new String[]{e.getKey(), arg})) - .collect(Collectors.toList()); - } - - @RunWith(Parameterized.class) - public static class TestIllegalFSAccess { - private final String apocProcedure; - private final String exportMethod; - - public TestIllegalFSAccess(String exportMethod, String exportMethodArguments) { - this.apocProcedure = "apoc.load." + exportMethod + exportMethodArguments; - this.exportMethod = exportMethod; - } - - @Parameterized.Parameters - public static Collection data() { - return LoadCoreSecurityTest.data(); - } - - @Test - public void testIllegalFSAccessWithImportDisabled() { - ApocConfig.apocConfig().setProperty(ApocConfig.APOC_IMPORT_FILE_ENABLED, false); - final String message = apocProcedure + " should throw an exception"; - try { - db.executeTransactionally("CALL " + apocProcedure, - Map.of("fileName", "./hello"), - Result::resultAsString); - fail(message); - } catch (Exception e) { - TestUtil.assertError(e, ApocConfig.LOAD_FROM_FILE_ERROR, RuntimeException.class, apocProcedure); - } - } - - @Test - public void testIllegalFSAccessWithImportEnabled() { - final String message = apocProcedure + " should throw an exception"; - final String fileName = SensitivePathGenerator.etcPasswd(db).getLeft(); - ApocConfig.apocConfig().setProperty(ApocConfig.APOC_IMPORT_FILE_ENABLED, true); - ApocConfig.apocConfig().setProperty(ApocConfig.APOC_IMPORT_FILE_USE_NEO4J_CONFIG, true); - ApocConfig.apocConfig().setProperty(ApocConfig.APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM, false); - try { - db.executeTransactionally("CALL " + apocProcedure, - Map.of("fileName", fileName), - Result::resultAsString); - fail(message); - } catch (Exception e) { - TestUtil.assertError(e, String.format(FileUtils.ERROR_READ_FROM_FS_NOT_ALLOWED, fileName), RuntimeException.class, apocProcedure); - } - } - - @Test - public void testReadSensitiveFileWorks() { - // as we're defining ApocConfig.APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM to true - // and ApocConfig.APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM to false the next call should work - final String fileName = SensitivePathGenerator.etcPasswd(db).getLeft(); - ApocConfig.apocConfig().setProperty(ApocConfig.APOC_IMPORT_FILE_ENABLED, true); - ApocConfig.apocConfig().setProperty(ApocConfig.APOC_IMPORT_FILE_USE_NEO4J_CONFIG, false); - ApocConfig.apocConfig().setProperty(ApocConfig.APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM, true); - try { - db.executeTransactionally("CALL " + apocProcedure, - Map.of("fileName", fileName), - Result::resultAsString); - if (ALLOWED_EXCEPTIONS.containsKey(exportMethod)) { - fail("Expected to fail by throwing the following exception: " + ALLOWED_EXCEPTIONS.get(exportMethod)); - } - } catch (Exception e) { - if (ALLOWED_EXCEPTIONS.containsKey(exportMethod)) { - assertEquals(ALLOWED_EXCEPTIONS.get(exportMethod), ExceptionUtils.getRootCause(e).getClass()); - } - } - } - } -} \ No newline at end of file diff --git a/core/src/test/java/apoc/load/LoadJsonTest.java b/core/src/test/java/apoc/load/LoadJsonTest.java index 97c9d26bd..5a8a6fbb4 100644 --- a/core/src/test/java/apoc/load/LoadJsonTest.java +++ b/core/src/test/java/apoc/load/LoadJsonTest.java @@ -85,8 +85,7 @@ public static void stopServer() { public DbmsRule db = new ImpermanentDbmsRule() .withSetting(GraphDatabaseSettings.memory_tracking, true) .withSetting(GraphDatabaseSettings.tx_state_memory_allocation, OFF_HEAP) - .withSetting(GraphDatabaseSettings.tx_state_max_off_heap_memory, BYTES.parse("1G")) - .withSetting(GraphDatabaseInternalSettings.cypher_ip_blocklist, List.of(new IPAddressString("127.168.0.0/8"))); + .withSetting(GraphDatabaseSettings.tx_state_max_off_heap_memory, BYTES.parse("1G")); @Before public void setUp() { apocConfig().setProperty(APOC_IMPORT_FILE_ENABLED, true); @@ -131,21 +130,7 @@ private void commonAssertionsLoadJsonMulti(Result result) { assertEquals(map("bar", asList(4L, 5L, 6L)), row.get("value")); assertFalse(result.hasNext()); } - - @Test public void testLoadJsonFromBlockedIpRange() { - var protocols = List.of("https", "http", "ftp"); - - for (var protocol: protocols) { - QueryExecutionException e = assertThrows(QueryExecutionException.class, - () -> testCall(db, - "CALL apoc.load.json('" + protocol + "://127.168.0.0/test.csv')", - map(), - (r) -> {} - ) - ); - assertTrue(e.getMessage().contains("access to /127.168.0.0 is blocked via the configuration property internal.dbms.cypher_ip_blocklist")); - } - } + @Test public void testLoadMultiJsonPaths() { URL url = ClassLoader.getSystemResource("multi.json"); testResult(db, "CALL apoc.load.json($url,'$')",map("url",url.toString()), // 'file:map.json' YIELD value RETURN value diff --git a/core/src/test/java/apoc/load/XmlTest.java b/core/src/test/java/apoc/load/XmlTest.java index 730fee147..26121caf6 100644 --- a/core/src/test/java/apoc/load/XmlTest.java +++ b/core/src/test/java/apoc/load/XmlTest.java @@ -63,8 +63,7 @@ public class XmlTest { public static final String FILE_SHORTENED = "src/test/resources/xml/humboldt_soemmering01_1791.TEI-P5-shortened.xml"; @Rule - public DbmsRule db = new ImpermanentDbmsRule() - .withSetting(GraphDatabaseInternalSettings.cypher_ip_blocklist, List.of(new IPAddressString("127.0.0.0/8"))); + public DbmsRule db = new ImpermanentDbmsRule(); @Before public void setUp() { @@ -223,18 +222,6 @@ public void testLoadXmlXpathReturnBookFromBookTitle () { }); } - @Test - public void testLoadXmlWithBlockedIP () { - QueryExecutionException e = assertThrows(QueryExecutionException.class, - () -> testCall(db, - "CALL apoc.load.xml('http://127.0.0.0/fake.xml') yield value as result", - map(), - (r) -> {} - ) - ); - TestCase.assertTrue(e.getMessage().contains("access to /127.0.0.0 is blocked via the configuration property internal.dbms.cypher_ip_blocklist")); - } - @Test public void testLoadXmlXpathBooKsFromGenre () { testResult(db, "CALL apoc.load.xml('" + TestUtil.getUrlFileName("xml/books.xml") + "', '/catalog/book[genre=\"Computer\"]') yield value as result", diff --git a/test-utils/src/main/java/apoc/util/FileTestUtil.java b/test-utils/src/main/java/apoc/util/FileTestUtil.java index 2e9cec412..59d6cdc99 100644 --- a/test-utils/src/main/java/apoc/util/FileTestUtil.java +++ b/test-utils/src/main/java/apoc/util/FileTestUtil.java @@ -19,11 +19,15 @@ package apoc.util; import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.UUID; import static org.junit.Assert.assertEquals; public class FileTestUtil { - + public static void assertStreamEquals(File directoryExpected, String fileName, String actualText) { String expectedText = TestUtil.readFileToString(new File(directoryExpected, fileName)); String[] actualArray = actualText.split("\n"); @@ -33,4 +37,12 @@ public static void assertStreamEquals(File directoryExpected, String fileName, S assertEquals(JsonUtil.parse(expectArray[i],null, Object.class), JsonUtil.parse(actualArray[i],null, Object.class)); } } + + public static Path createTempFolder() { + try { + return Files.createTempDirectory( UUID.randomUUID().toString() ); + } catch (IOException e) { + throw new RuntimeException(e); + } + } } diff --git a/test-utils/src/main/java/apoc/util/SensitivePathGenerator.java b/test-utils/src/main/java/apoc/util/SensitivePathGenerator.java index fbeaa8792..9860f070c 100644 --- a/test-utils/src/main/java/apoc/util/SensitivePathGenerator.java +++ b/test-utils/src/main/java/apoc/util/SensitivePathGenerator.java @@ -33,20 +33,34 @@ private SensitivePathGenerator() {} /** * It will return an instance of Pair where first is the relative path * and other the absolute path of "etc/passwd" - * @param db * @return */ - public static Pair etcPasswd(GraphDatabaseAPI db) { - return base(db, "/etc/passwd"); + public static Pair etcPasswd() { + return base("/etc/passwd"); } - private static Pair base(GraphDatabaseAPI db, String path) { - final Path dbPath = db.databaseLayout().databaseDirectory(); - final String relativeFileName = IntStream.range(0, dbPath.getNameCount()) - .mapToObj(i -> "..") - .collect(Collectors.joining("/")) + path; - final String absoluteFileName = Paths.get(relativeFileName) - .toAbsolutePath().normalize().toString(); - return Pair.of(relativeFileName, absoluteFileName); + private static Pair base(String path) { + try { + System.out.println("Paths.get(\"..\", System.getProperty(\"coreDir\")).toFile().getCanonicalPath() = " + Paths.get("..", System.getProperty("coreDir")).toFile().getCanonicalPath()); + Path path1 = Paths.get("").toAbsolutePath(); + System.out.println("path1 = " + path1); + final String relativeFileName1 = IntStream.range(0, path1.getNameCount()) +// final String relativeFileName1 = IntStream.range(0, Paths.get("..", System.getProperty("coreDir")).toAbsolutePath().getNameCount()) + .mapToObj(i -> "..") + .collect(Collectors.joining("/")) + path; + final String absoluteFileName1 = Paths.get(relativeFileName1) + .toAbsolutePath().normalize().toString(); + + +// final Path dbPath = db.databaseLayout().databaseDirectory(); +// final String relativeFileName = IntStream.range(0, dbPath.getNameCount()) +// .mapToObj(i -> "..") +// .collect(Collectors.joining("/")) + path; +// final String absoluteFileName = Paths.get(relativeFileName) +// .toAbsolutePath().normalize().toString(); + return Pair.of(relativeFileName1, absoluteFileName1); + } catch (Exception e) { + throw new RuntimeException(e); + } } } \ No newline at end of file