Skip to content

ExtractVirtualFileSystemResources does not set correct permissions #46

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ih0r-d wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

ExtractVirtualFileSystemResources does not set correct permissions #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7cee664
feat(vfs): make file permissions explicit and apply them during extra…
ih0r-d Jan 18, 2026
6a54331
refactor: reuse shared permissions constants
ih0r-d Jan 19, 2026
0c7bdc9
fix: restore executable permissions for filelist entries and formatting
ih0r-d Jan 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 128 additions & 12 deletions ...vm.python.embedding/src/main/java/org/graalvm/python/embedding/VirtualFileSystemImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -102,6 +105,12 @@

final class VirtualFileSystemImpl implements FileSystem, AutoCloseable {

private static final Set<PosixFilePermission> DEFAULT_FILE_PERMISSIONS = PosixFilePermissions
.fromString("rw-r--r--"); // 0644

private static final Set<PosixFilePermission> DEFAULT_DIR_PERMISSIONS = PosixFilePermissions
.fromString("rwxr-xr-x"); // 0755

private static final Logger LOGGER = Logger.getLogger(VirtualFileSystem.class.getName());

static {
Expand Down Expand Up @@ -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<PosixFilePermission> permissions;

private BaseEntry(String platformPath) {
private BaseEntry(String platformPath, Set<PosixFilePermission> permissions) {
this.platformPath = platformPath;
this.permissions = permissions;
}

String getPlatformPath() {
Expand All @@ -198,14 +209,87 @@ String getResourcePath() {
static AssertionError throwUnexpectedSubclass() {
throw new AssertionError("Unexpected subclass of sealed DirEntry");
}

public Set<PosixFilePermission> getPermissions() {
return permissions;
}
}

private record FilelistEntry(EntryType type, String resourcePath, Set<PosixFilePermission> 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<PosixFilePermission> permissions = isExecutable(line)
? DEFAULT_DIR_PERMISSIONS // 0755
: DEFAULT_FILE_PERMISSIONS; // 0644

return new FilelistEntry(EntryType.FILE, line, permissions);
}

// v2 format: <type> <mode> <path>
String[] parts = line.split("\\s+", 3);
if (parts.length != 3) {
throw new IllegalArgumentException("Invalid fileslist entry (expected: <type> <mode> <path>): " + 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<PosixFilePermission> 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<PosixFilePermission> 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<FileEntry> toExtract;

public FileEntry(String path) {
super(path);
public FileEntry(String path, Set<PosixFilePermission> permissions) {
super(path, permissions);
}

private byte[] getData() throws IOException {
Expand All @@ -221,8 +305,8 @@ private byte[] getData() throws IOException {
private final class DirEntry extends BaseEntry {
List<BaseEntry> entries = new ArrayList<>();

DirEntry(String platformPath) {
super(platformPath);
DirEntry(String platformPath, Set<PosixFilePermission> permissions) {
super(platformPath, permissions);
}
}

Expand Down Expand Up @@ -462,21 +546,24 @@ private void initEntries() {
String venvPath = absoluteResourcePath(vfsRoot, VFS_VENV);
List<URL> filelistUrls = getFilelistURLs(filelistPath);
boolean hasNativeFiles = false;

fine("VFS fileslistPath = %s", filelistPath);
for (URL url : filelistUrls) {
try (InputStream stream = url.openStream()) {
if (stream == null) {
warn("VFS.initEntries: could not read resource %s", filelistPath);
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;
Expand All @@ -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) {
Expand All @@ -514,7 +601,14 @@ private void initEntries() {

assert parent != null;
if (!platformPath.endsWith(PLATFORM_SEPARATOR)) {
FileEntry fileEntry = new FileEntry(platformPath);
Set<PosixFilePermission> 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);
}
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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);
}
}
}
Expand Down
89 changes: 57 additions & 32 deletions ...thon.embedding/src/test/java/org/graalvm/python/embedding/test/VirtualFileSystemTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {

Expand Down Expand Up @@ -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,
Expand All @@ -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)),
Expand Down Expand Up @@ -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");
}
}
}

Expand All @@ -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");
}
}
}

Expand Down