fix: security path accessor

MEFRREEX · MEFRREEX · commit 543f6aeece48 · 2025-08-16T23:59:10.000+04:00
diff --git a/api/src/main/java/com/instancify/scriptify/api/script/security/ScriptSecurityManager.java b/api/src/main/java/com/instancify/scriptify/api/script/security/ScriptSecurityManager.java
@@ -22,6 +22,13 @@ public interface ScriptSecurityManager {
      */
     void setSecurityMode(boolean securityMode);
 
+    /**
+     * Receives security file system.
+     *
+     * @return Security file system
+     */
+    SecurityFileSystem getFileSystem();
+
     /**
      * Receives security path accessor.
      *
diff --git a/api/src/main/java/com/instancify/scriptify/api/script/security/SecurityFileSystem.java b/api/src/main/java/com/instancify/scriptify/api/script/security/SecurityFileSystem.java
@@ -0,0 +1,34 @@
+package com.instancify.scriptify.api.script.security;
+
+import java.io.File;
+import java.nio.file.Path;
+
+/**
+ * Provides secure access to files and paths, ensuring
+ * all operations are restricted to the configured base path.
+ */
+public interface SecurityFileSystem {
+
+    /**
+     * Returns a secure Path inside the base path.
+     *
+     * @param path the path string to resolve
+     * @return a normalized and secure Path
+     * @throws SecurityException if the path is outside basePath or not accessible
+     */
+    Path getPath(String path);
+
+    /**
+     * Returns a secure File inside the base path.
+     *
+     * @param path the path string to resolve
+     * @return a normalized and secure File
+     * @throws SecurityException if the path is outside basePath or not accessible
+     */
+    File getFile(String path);
+
+    /**
+     * Returns the base path for this file system.
+     */
+    Path getBasePath();
+}
diff --git a/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionDeleteFile.java b/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionDeleteFile.java
@@ -30,14 +30,14 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr
         }
 
         if (args.length == 1) {
-            return new File(filePath).delete();
+            return script.getSecurityManager().getFileSystem().getFile(filePath).delete();
         }
 
         if (!(args[1].getValue() instanceof Boolean recursive)) {
             throw new ScriptFunctionArgTypeException(Boolean.class, args[1].getType());
         }
 
-        File file = new File(filePath);
+        File file = script.getSecurityManager().getFileSystem().getFile(filePath);
         if (recursive) {
             return deleteDirectoryRecursively(file);
         } else {
diff --git a/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionDownloadFromUrl.java b/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionDownloadFromUrl.java
@@ -11,7 +11,8 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.URL;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.nio.file.Files;
 
 /**
@@ -37,11 +38,11 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr
             throw new ScriptFunctionArgTypeException(String.class, args[1].getType());
         }
 
-        try (InputStream in = new URL(url).openStream()) {
-            File targetPath = new File(filePath);
+        try (InputStream in = new URI(url).toURL().openStream()) {
+            File targetPath = script.getSecurityManager().getFileSystem().getFile(filePath);
             Files.copy(in, targetPath.toPath());
-        } catch (IOException e) {
-            e.printStackTrace();
+        } catch (IOException | URISyntaxException e) {
+            throw new RuntimeException(e);
         }
 
         return null;
diff --git a/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionExistsFile.java b/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionExistsFile.java
@@ -25,7 +25,7 @@ public class ScriptFunctionExistsFile implements ScriptFunction {
     public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws ScriptFunctionException {
         if (args.length == 1) {
             if (args[0].getValue() instanceof String filePath) {
-                return Files.exists(Path.of(filePath));
+                return Files.exists(script.getSecurityManager().getFileSystem().getPath(filePath));
             } else {
                 throw new ScriptFunctionArgTypeException(String.class, args[0].getType());
             }
diff --git a/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionListFiles.java b/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionListFiles.java
@@ -9,8 +9,8 @@
 import org.jetbrains.annotations.NotNull;
 
 import java.io.File;
-import java.nio.file.Paths;
 import java.util.Arrays;
+import java.util.Objects;
 
 /**
  * Represents a function to get all files in a folder
@@ -26,9 +26,9 @@ public class ScriptFunctionListFiles implements ScriptFunction {
     public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws ScriptFunctionException {
         if (args.length == 1) {
             if (args[0].getValue() instanceof String filePath) {
-                File folder = Paths.get(filePath).toAbsolutePath().toFile();
+                File folder = script.getSecurityManager().getFileSystem().getFile(filePath);
                 if (folder.isDirectory()) {
-                    return Arrays.stream(folder.listFiles()).map(File::getAbsolutePath).toList();
+                    return Arrays.stream(Objects.requireNonNull(folder.listFiles())).map(File::getAbsolutePath).toList();
                 } else {
                     throw new ScriptFunctionException("File is not a folder");
                 }
diff --git a/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionMoveFile.java b/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionMoveFile.java
@@ -33,7 +33,7 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr
             throw new ScriptFunctionArgTypeException(String.class, args[1].getType());
         }
 
-        File fileToMove = new File(originalFilePath);
-        return fileToMove.renameTo(new File(targetFilePath));
+        File fileToMove = script.getSecurityManager().getFileSystem().getFile(originalFilePath);
+        return fileToMove.renameTo(script.getSecurityManager().getFileSystem().getFile(targetFilePath));
     }
 }
diff --git a/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionReadFile.java b/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionReadFile.java
@@ -27,7 +27,7 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr
         if (args.length == 1) {
             if (args[0].getValue() instanceof String filePath) {
                 try {
-                    return Files.readString(Path.of(filePath));
+                    return Files.readString(script.getSecurityManager().getFileSystem().getPath(filePath));
                 } catch (IOException e) {
                     throw new ScriptFunctionException(e);
                 }
diff --git a/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionWriteFile.java b/core/src/main/java/com/instancify/scriptify/core/script/function/impl/file/ScriptFunctionWriteFile.java
@@ -26,7 +26,7 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr
         if (args.length == 2) {
             if (args[0].getValue() instanceof String filePath && args[1].getValue() instanceof String fileContent) {
                 try {
-                    return Files.writeString(script.getSecurityManager().getPathAccessor().getAccessiblePath(filePath), fileContent);
+                    return Files.writeString(script.getSecurityManager().getFileSystem().getPath(filePath), fileContent);
                 } catch (IOException e) {
                     throw new ScriptFunctionException(e);
                 }
diff --git a/security/src/main/java/com/instancify/scriptify/security/SecurityFileSystemImpl.java b/security/src/main/java/com/instancify/scriptify/security/SecurityFileSystemImpl.java
@@ -0,0 +1,31 @@
+package com.instancify.scriptify.security;
+
+import com.instancify.scriptify.api.script.security.SecurityFileSystem;
+import com.instancify.scriptify.api.script.security.SecurityPathAccessor;
+
+import java.io.File;
+import java.nio.file.Path;
+
+public class SecurityFileSystemImpl implements SecurityFileSystem {
+
+    private final SecurityPathAccessor pathAccessor;
+
+    public SecurityFileSystemImpl(SecurityPathAccessor pathAccessor) {
+        this.pathAccessor = pathAccessor;
+    }
+
+    @Override
+    public Path getPath(String path) {
+        return pathAccessor.getAccessiblePath(path);
+    }
+
+    @Override
+    public File getFile(String path) {
+        return this.getPath(path).toFile();
+    }
+
+    @Override
+    public Path getBasePath() {
+        return pathAccessor.getBasePath();
+    }
+}
diff --git a/security/src/main/java/com/instancify/scriptify/security/SecurityPathAccessorImpl.java b/security/src/main/java/com/instancify/scriptify/security/SecurityPathAccessorImpl.java
@@ -57,18 +57,31 @@ public void setBasePath(Path basePath) {
     }
 
     /**
-     * Returns a path that is safe to access according to security rules. If the path is not accessible,
-     * it returns a path relative to the base path with ':' characters removed to prevent potential path traversal attacks.
+     * Returns a path that is safe to access according to security rules.
+     * If the path is accessible via exclusions, returns the normalized path.
+     * If the path is not accessible, creates a safe path within basePath by cleaning the path from invalid characters.
      *
      * @param path The path string to be checked and possibly modified
-     * @return A Path object representing the accessible path or a sanitized version if not accessible
+     * @return A Path object representing the accessible path or a path within base directory
      */
     @Override
     public Path getAccessiblePath(String path) {
         if (this.isAccessible(path)) {
-            return Path.of(path);
+            // Path is in exclusions - return it normalized
+            return Paths.get(path).normalize().toAbsolutePath();
         }
-        return Path.of(basePath.toString(), path.replaceAll(":", ""));
+
+        // Path is not accessible - create safe path within basePath
+        // We need to manually combine paths because resolve() ignores basePath for absolute paths
+        Path safePath = Paths.get(basePath.toString(), path.replace(":", "")).normalize();
+
+        // CRITICAL: Ensure the result stays within basePath boundaries
+        if (!safePath.startsWith(basePath)) {
+            // If path tries to escape basePath (e.g., "../"), return basePath itself
+            return basePath;
+        }
+
+        return safePath;
     }
 
     /**
@@ -83,12 +96,35 @@ public boolean isAccessible(String path) {
             return true;
         }
 
+        // Normalize the path to resolve .. and . components to prevent path traversal
+        Path normalizedPath;
+        try {
+            normalizedPath = Paths.get(path).normalize().toAbsolutePath();
+        } catch (Exception e) {
+            return false;
+        }
+
+        // Check both original and normalized path against exclusions for compatibility
+        String normalizedPathString = normalizedPath.toString();
+
         // Search all exclusions and check that the path is excluded
         for (SecurityExclude exclude : securityManager.getExcludes()) {
             if (exclude instanceof PathSecurityExclude) {
+                // Check original path first
                 if (exclude.isExcluded(path)) {
                     return true;
                 }
+
+                // Check normalized path
+                if (exclude.isExcluded(normalizedPathString)) {
+                    return true;
+                }
+
+                // Check with forward slashes for cross-platform compatibility
+                String pathWithForwardSlashes = normalizedPathString.replace('\\', '/');
+                if (exclude.isExcluded(pathWithForwardSlashes)) {
+                    return true;
+                }
             }
         }
 
diff --git a/security/src/main/java/com/instancify/scriptify/security/StandardSecurityManager.java b/security/src/main/java/com/instancify/scriptify/security/StandardSecurityManager.java
@@ -12,6 +12,7 @@ public class StandardSecurityManager implements ScriptSecurityManager {
     private boolean securityMode;
     private final Set<SecurityExclude> excludes = new HashSet<>();
     private final SecurityPathAccessor pathAccessor = new SecurityPathAccessorImpl(this);
+    private final SecurityFileSystemImpl fileSystem = new SecurityFileSystemImpl(pathAccessor);
 
     @Override
     public boolean getSecurityMode() {
@@ -23,6 +24,11 @@ public void setSecurityMode(boolean securityMode) {
         this.securityMode = securityMode;
     }
 
+    @Override
+    public SecurityFileSystemImpl getFileSystem() {
+        return fileSystem;
+    }
+
     @Override
     public SecurityPathAccessor getPathAccessor() {
         return pathAccessor;

Original file line number	Diff line number	Diff line change
`@@ -30,14 +30,14 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr`
`30`	`30`	`}`
`31`	`31`
`32`	`32`	`if (args.length == 1) {`
`33`		`- return new File(filePath).delete();`
	`33`	`+ return script.getSecurityManager().getFileSystem().getFile(filePath).delete();`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`if (!(args[1].getValue() instanceof Boolean recursive)) {`
`37`	`37`	`throw new ScriptFunctionArgTypeException(Boolean.class, args[1].getType());`
`38`	`38`	`}`
`39`	`39`
`40`		`- File file = new File(filePath);`
	`40`	`+ File file = script.getSecurityManager().getFileSystem().getFile(filePath);`
`41`	`41`	`if (recursive) {`
`42`	`42`	`return deleteDirectoryRecursively(file);`
`43`	`43`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ public class ScriptFunctionExistsFile implements ScriptFunction {`
`25`	`25`	`public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws ScriptFunctionException {`
`26`	`26`	`if (args.length == 1) {`
`27`	`27`	`if (args[0].getValue() instanceof String filePath) {`
`28`		`- return Files.exists(Path.of(filePath));`
	`28`	`+ return Files.exists(script.getSecurityManager().getFileSystem().getPath(filePath));`
`29`	`29`	`} else {`
`30`	`30`	`throw new ScriptFunctionArgTypeException(String.class, args[0].getType());`
`31`	`31`	`}`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr`
`33`	`33`	`throw new ScriptFunctionArgTypeException(String.class, args[1].getType());`
`34`	`34`	`}`
`35`	`35`
`36`		`- File fileToMove = new File(originalFilePath);`
`37`		`- return fileToMove.renameTo(new File(targetFilePath));`
	`36`	`+ File fileToMove = script.getSecurityManager().getFileSystem().getFile(originalFilePath);`
	`37`	`+ return fileToMove.renameTo(script.getSecurityManager().getFileSystem().getFile(targetFilePath));`
`38`	`38`	`}`
`39`	`39`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr`
`27`	`27`	`if (args.length == 1) {`
`28`	`28`	`if (args[0].getValue() instanceof String filePath) {`
`29`	`29`	`try {`
`30`		`- return Files.readString(Path.of(filePath));`
	`30`	`+ return Files.readString(script.getSecurityManager().getFileSystem().getPath(filePath));`
`31`	`31`	`} catch (IOException e) {`
`32`	`32`	`throw new ScriptFunctionException(e);`
`33`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ public Object invoke(Script<?> script, ScriptFunctionArgument[] args) throws Scr`
`26`	`26`	`if (args.length == 2) {`
`27`	`27`	`if (args[0].getValue() instanceof String filePath && args[1].getValue() instanceof String fileContent) {`
`28`	`28`	`try {`
`29`		`- return Files.writeString(script.getSecurityManager().getPathAccessor().getAccessiblePath(filePath), fileContent);`
	`29`	`+ return Files.writeString(script.getSecurityManager().getFileSystem().getPath(filePath), fileContent);`
`30`	`30`	`} catch (IOException e) {`
`31`	`31`	`throw new ScriptFunctionException(e);`
`32`	`32`	`}`