diff --git a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java index 1bfa25c..582c1f1 100644 --- a/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java +++ b/org.graalvm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java @@ -40,6 +40,9 @@ */ package org.graalvm.python.embedding; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumSet; import org.graalvm.polyglot.io.FileSystem; import org.graalvm.python.embedding.VirtualFileSystem.HostIO; @@ -102,6 +105,12 @@ final class VirtualFileSystemImpl implements FileSystem, AutoCloseable { + private static final Set DEFAULT_FILE_PERMISSIONS = PosixFilePermissions + .fromString("rw-r--r--"); // 0644 + + private static final Set DEFAULT_DIR_PERMISSIONS = PosixFilePermissions + .fromString("rwxr-xr-x"); // 0755 + private static final Logger LOGGER = Logger.getLogger(VirtualFileSystem.class.getName()); static { @@ -180,11 +189,13 @@ private static String absoluteResourcePath(String... components) { private static final char RESOURCE_SEPARATOR_CHAR = '/'; private static final String RESOURCE_SEPARATOR = String.valueOf(RESOURCE_SEPARATOR_CHAR); - private abstract sealed class BaseEntry permits FileEntry, DirEntry { + private abstract sealed class BaseEntry permits DirEntry, FileEntry { final String platformPath; + private final Set permissions; - private BaseEntry(String platformPath) { + private BaseEntry(String platformPath, Set permissions) { this.platformPath = platformPath; + this.permissions = permissions; } String getPlatformPath() { @@ -198,14 +209,87 @@ String getResourcePath() { static AssertionError throwUnexpectedSubclass() { throw new AssertionError("Unexpected subclass of sealed DirEntry"); } + + public Set getPermissions() { + return permissions; + } + } + + private record FilelistEntry(EntryType type, String resourcePath, Set permissions) { + enum EntryType { + FILE, DIR + } + + static FilelistEntry parse(String line) { + if (line == null) { + return null; + } + + line = line.trim(); + if (line.isEmpty() || line.startsWith("#")) { + return null; + } + + // v1 format: only absolute path + if (line.startsWith("/")) { + Set permissions = isExecutable(line) + ? DEFAULT_DIR_PERMISSIONS // 0755 + : DEFAULT_FILE_PERMISSIONS; // 0644 + + return new FilelistEntry(EntryType.FILE, line, permissions); + } + + // v2 format: + String[] parts = line.split("\\s+", 3); + if (parts.length != 3) { + throw new IllegalArgumentException("Invalid fileslist entry (expected: ): " + line); + } + + EntryType type = switch (parts[0]) { + case "file" -> EntryType.FILE; + case "dir" -> EntryType.DIR; + default -> throw new IllegalArgumentException("Unknown fileslist entry type: " + parts[0]); + }; + + Set permissions = parsePermissions(parts[1]); + String resourcePath = parts[2]; + + if (!resourcePath.startsWith("/")) { + throw new IllegalArgumentException("Resource path must be absolute: " + resourcePath); + } + + return new FilelistEntry(type, resourcePath, permissions); + } + + private static Set parsePermissions(String mode) { + if (!mode.matches("[0-7]{4}")) { + throw new IllegalArgumentException("Invalid permission mode: " + mode); + } + return PosixFilePermissions.fromString(octalToSymbolic(mode)); + } + + private static String octalToSymbolic(String octal) { + StringBuilder sb = new StringBuilder(9); + for (int i = 1; i < 4; i++) { + int digit = octal.charAt(i) - '0'; + sb.append((digit & 4) != 0 ? 'r' : '-'); + sb.append((digit & 2) != 0 ? 'w' : '-'); + sb.append((digit & 1) != 0 ? 'x' : '-'); + } + return sb.toString(); + } + } + + private static boolean isExecutable(String resourcePath) { + return resourcePath.endsWith(".so") || resourcePath.endsWith(".sh") || resourcePath.contains("/bin/"); } private final class FileEntry extends BaseEntry { private byte[] data; private List toExtract; - public FileEntry(String path) { - super(path); + public FileEntry(String path, Set permissions) { + super(path, permissions); } private byte[] getData() throws IOException { @@ -221,8 +305,8 @@ private byte[] getData() throws IOException { private final class DirEntry extends BaseEntry { List entries = new ArrayList<>(); - DirEntry(String platformPath) { - super(platformPath); + DirEntry(String platformPath, Set permissions) { + super(platformPath, permissions); } } @@ -462,6 +546,8 @@ private void initEntries() { String venvPath = absoluteResourcePath(vfsRoot, VFS_VENV); List filelistUrls = getFilelistURLs(filelistPath); boolean hasNativeFiles = false; + + fine("VFS fileslistPath = %s", filelistPath); for (URL url : filelistUrls) { try (InputStream stream = url.openStream()) { if (stream == null) { @@ -469,14 +555,15 @@ private void initEntries() { return; } try (BufferedReader br = new BufferedReader(new InputStreamReader(stream, StandardCharsets.UTF_8))) { - String resourcePath; + String line; finest("VFS entries:"); - while ((resourcePath = br.readLine()) != null) { - if (resourcePath.isBlank()) { - // allow empty lines, some tools insert empty lines when concatenating files + while ((line = br.readLine()) != null) { + FilelistEntry meta = FilelistEntry.parse(line); + if (meta == null) { continue; } + String resourcePath = meta.resourcePath(); String projPath = absoluteResourcePath(vfsRoot, PROJ_DIR); if (!projWarning && resourcePath.startsWith(projPath)) { projWarning = true; @@ -499,7 +586,7 @@ private void initEntries() { if (genericEntry instanceof DirEntry de) { dirEntry = de; } else if (genericEntry == null) { - dirEntry = new DirEntry(dir); + dirEntry = new DirEntry(dir, DEFAULT_DIR_PERMISSIONS); vfsEntries.put(dirKey, dirEntry); finest(" %s", dirEntry.getResourcePath()); if (parent != null) { @@ -514,7 +601,14 @@ private void initEntries() { assert parent != null; if (!platformPath.endsWith(PLATFORM_SEPARATOR)) { - FileEntry fileEntry = new FileEntry(platformPath); + Set permissions = meta.permissions(); + if (isExecutable(platformPath)) { + permissions = EnumSet.copyOf(permissions); + permissions.add(PosixFilePermission.OWNER_EXECUTE); + permissions.add(PosixFilePermission.GROUP_EXECUTE); + permissions.add(PosixFilePermission.OTHERS_EXECUTE); + } + FileEntry fileEntry = new FileEntry(platformPath, permissions); if (extractFilter != null && extractFilter.test(Paths.get(platformPath))) { fileEntry.toExtract = List.of(fileEntry); } @@ -934,6 +1028,16 @@ private Path getExtractedPath(BaseEntry entry) { } } + private void applyPermissions(BaseEntry entry, Path path) { + try { + if (Files.getFileStore(path).supportsFileAttributeView("posix")) { + Files.setPosixFilePermissions(path, entry.getPermissions()); + } + } catch (IOException e) { + warn("Failed to set permissions for %s: %s", path, e.getMessage()); + } + } + private Path extractSingleFile(FileEntry toExtract) throws IOException { /* * Remove the mountPoint(X) (e.g. "graalpy_vfs(x)") prefix if given. Method @@ -952,8 +1056,18 @@ private Path extractSingleFile(FileEntry toExtract) throws IOException { } Files.createDirectories(parent); + // apply permissions to parent dirs + BaseEntry parentEntry = getEntry(parent); + if (parentEntry != null) { + applyPermissions(parentEntry, parent); + } + // write data extracted file Files.write(extractedPath, toExtract.getData()); + + // apply permissions to extracted file + applyPermissions(toExtract, extractedPath); + finest("extracted '%s' -> '%s'", toExtract.getPlatformPath(), extractedPath); } return extractedPath; @@ -970,6 +1084,7 @@ void extractResources(Path externalResourceDirectory) throws IOException { Path destFile = externalResourceDirectory.resolve(Path.of(resourcePath.substring(vfsRoot.length() + 2))); if (entry instanceof DirEntry) { Files.createDirectories(destFile); + applyPermissions(entry, destFile); } else { assert entry instanceof FileEntry; Path parent = destFile.getParent(); @@ -978,6 +1093,7 @@ void extractResources(Path externalResourceDirectory) throws IOException { } finest("VFS.extractResources '%s' -> '%s'", resourcePath, destFile); Files.write(destFile, readResource(resourcePath)); + applyPermissions(entry, destFile); } } } diff --git a/org.graalvm.python.embedding/src/test/java/org/graalvm/python/embedding/test/VirtualFileSystemTest.java b/org.graalvm.python.embedding/src/test/java/org/graalvm/python/embedding/test/VirtualFileSystemTest.java index 5d75e87..047e461 100644 --- a/org.graalvm.python.embedding/src/test/java/org/graalvm/python/embedding/test/VirtualFileSystemTest.java +++ b/org.graalvm.python.embedding/src/test/java/org/graalvm/python/embedding/test/VirtualFileSystemTest.java @@ -41,12 +41,17 @@ package org.graalvm.python.embedding.test; -import org.graalvm.polyglot.io.FileSystem; -import org.graalvm.python.embedding.VirtualFileSystem; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.function.Executable; +import static org.graalvm.python.embedding.VirtualFileSystem.HostIO.NONE; +import static org.graalvm.python.embedding.VirtualFileSystem.HostIO.READ; +import static org.graalvm.python.embedding.VirtualFileSystem.HostIO.READ_WRITE; +import static org.graalvm.python.embedding.test.TestUtils.IS_WINDOWS; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import java.io.File; import java.io.IOException; @@ -82,18 +87,12 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Stream; - -import static org.graalvm.python.embedding.VirtualFileSystem.HostIO.NONE; -import static org.graalvm.python.embedding.VirtualFileSystem.HostIO.READ; -import static org.graalvm.python.embedding.VirtualFileSystem.HostIO.READ_WRITE; -import static org.graalvm.python.embedding.test.TestUtils.IS_WINDOWS; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; +import org.graalvm.polyglot.io.FileSystem; +import org.graalvm.python.embedding.VirtualFileSystem; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; public class VirtualFileSystemTest { @@ -423,7 +422,7 @@ private static void checkAccessVFS(FileSystem fs, String pathPrefix) throws IOEx assertThrows(SecurityException.class, () -> fs.checkAccess(Path.of(pathPrefix, "extractme"), Set.of(AccessMode.WRITE), LinkOption.NOFOLLOW_LINKS)); fs.checkAccess(Path.of(pathPrefix, "extractme"), Set.of(AccessMode.READ)); - // even though extracted -> FS is read-only and we are limiting the access to + // even though extracted -> FS is read-only, and we are limiting the access to // read-only also // for extracted files assertThrows(IOException.class, @@ -434,18 +433,23 @@ private static void checkAccessVFS(FileSystem fs, String pathPrefix) throws IOEx assertThrows(NoSuchFileException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exits", "extractme"), Set.of(AccessMode.READ))); + // write must still be forbidden in VFS checkException(SecurityException.class, () -> fs.checkAccess(Path.of(pathPrefix, "SomeFile"), Set.of(AccessMode.WRITE)), "write access should not be possible with VFS"); + + // executable file + fs.checkAccess(Path.of(pathPrefix, "bin", "exec.sh"), Set.of(AccessMode.EXECUTE)); + + // non-executable file checkException(SecurityException.class, - () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exist"), Set.of(AccessMode.WRITE)), - "execute access should not be possible with VFS"); - checkException(SecurityException.class, - () -> fs.checkAccess(Path.of(pathPrefix, "SomeFile"), Set.of(AccessMode.EXECUTE)), - "execute access should not be possible with VFS"); + () -> fs.checkAccess(Path.of(pathPrefix, "bin", "nonexec.txt"), Set.of(AccessMode.EXECUTE)), + "execute access should not be possible for non-executable file"); + + // nonexistent paths checkException(SecurityException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exist"), Set.of(AccessMode.EXECUTE)), - "execute access should not be possible with VFS"); + "execute access should not be possible for non-existent path"); checkException(NoSuchFileException.class, () -> fs.checkAccess(Path.of(pathPrefix, "does-not-exits"), Set.of(AccessMode.READ)), @@ -785,9 +789,19 @@ public void libsExtract() throws Exception { checkExtractedFile(p, null); Path extractedRoot = p.getParent().getParent().getParent(); - checkExtractedFile(extractedRoot.resolve("src/package1.libs/fake-dependency1.so"), null); - checkExtractedFile(extractedRoot.resolve("src/package1.libs/fake-dependency2.so.2"), null); - assertFalse(Files.exists(extractedRoot.resolve("src/package2.libs/not-extracted.so"))); + Path dep1 = extractedRoot.resolve("src/package1.libs/fake-dependency1.so"); + Path dep2 = extractedRoot.resolve("src/package1.libs/fake-dependency2.so.2"); + Path notExtracted = extractedRoot.resolve("src/package2.libs/not-extracted.so"); + + checkExtractedFile(dep1, null); + checkExtractedFile(dep2, null); + assertFalse(Files.exists(notExtracted)); + + if (!IS_WINDOWS) { + assertTrue(Files.isExecutable(p), "main .so should be executable"); + assertTrue(Files.isExecutable(dep1), "dependency .so should be executable"); + assertTrue(Files.isExecutable(dep2), "dependency .so should be executable"); + } } } @@ -803,10 +817,21 @@ public void libsExtractCustomFilter() throws Exception { checkExtractedFile(p, null); Path extractedRoot = p.getParent().getParent().getParent(); - checkExtractedFile(extractedRoot.resolve("src/pkg1/__init__.py"), null); - checkExtractedFile(extractedRoot.resolve("src/package1.libs/fake-dependency1.so"), null); - checkExtractedFile(extractedRoot.resolve("src/package1.libs/fake-dependency2.so.2"), null); - assertFalse(Files.exists(extractedRoot.resolve("src/package2.libs/not-extracted.so"))); + Path initPy = extractedRoot.resolve("src/pkg1/__init__.py"); + Path dep1 = extractedRoot.resolve("src/package1.libs/fake-dependency1.so"); + Path dep2 = extractedRoot.resolve("src/package1.libs/fake-dependency2.so.2"); + Path notExtracted = extractedRoot.resolve("src/package2.libs/not-extracted.so"); + + checkExtractedFile(initPy, null); + checkExtractedFile(dep1, null); + checkExtractedFile(dep2, null); + assertFalse(Files.exists(notExtracted)); + + if (!IS_WINDOWS) { + assertFalse(Files.isExecutable(initPy), "__init__.py should not be executable"); + assertTrue(Files.isExecutable(dep1), "dependency .so should be executable"); + assertTrue(Files.isExecutable(dep2), "dependency .so should be executable"); + } } }