diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeActionTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeActionTest.java new file mode 100644 index 00000000000..03052fecee9 --- /dev/null +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeActionTest.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you 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 org.apache.logging.log4j.core.appender.rolling.action; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.test.BasicConfigurationFactory; +import org.apache.logging.log4j.core.util.FileUtils; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Tests the {@code PosixViewAttributeAction} class. + */ +class PosixViewAttributeActionTest { + + @BeforeAll + static void beforeClass() { + assumeTrue(FileUtils.isFilePosixAttributeViewSupported()); + } + + @Test + void testSymbolicLinksAreNotFollowed(@TempDir final Path tempDir) throws Exception { + // A file outside the scanned directory, that the action must not touch. + final Path outsider = tempDir.resolve("outsider.txt"); + Files.write(outsider, "secret".getBytes(StandardCharsets.UTF_8)); + Files.setPosixFilePermissions(outsider, PosixFilePermissions.fromString("rw-------")); + + final Path baseDir = Files.createDirectory(tempDir.resolve("logs")); + final Path regularFile = baseDir.resolve("app-1.log"); + Files.write(regularFile, "log".getBytes(StandardCharsets.UTF_8)); + Files.setPosixFilePermissions(regularFile, PosixFilePermissions.fromString("rw-------")); + Files.createSymbolicLink(baseDir.resolve("app-2.log"), outsider); + + final Configuration config = new BasicConfigurationFactory().new BasicConfiguration(); + final PosixViewAttributeAction action = PosixViewAttributeAction.newBuilder() + .setBasePath(baseDir.toString()) + .setFollowLinks(false) + .setMaxDepth(1) + .setPathConditions(PathCondition.EMPTY_ARRAY) + .setConfiguration(config) + .setFilePermissionsString("rw-rw-rw-") + .build(); + + action.execute(); + + assertEquals( + "rw-rw-rw-", + PosixFilePermissions.toString(Files.getPosixFilePermissions(regularFile)), + "regular file should have been updated"); + assertEquals( + "rw-------", + PosixFilePermissions.toString(Files.getPosixFilePermissions(outsider)), + "symbolic link target should have been left alone"); + } +} diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java index 35ec485703c..e2791200304 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/FileUtilsTest.java @@ -20,11 +20,14 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.io.File; import java.io.IOException; import java.net.URI; import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermissions; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; @@ -87,6 +90,28 @@ void testFileFromUriWithSpacesAndPlusCharactersInName() throws Exception { assertTrue(file.exists(), "file exists"); } + @Test + void testDefineFilePosixAttributeViewDoesNotFollowSymbolicLinks(@TempDir final Path tempDir) throws Exception { + assumeTrue(FileUtils.isFilePosixAttributeViewSupported()); + + final Path target = tempDir.resolve("target.txt"); + Files.createFile(target); + Files.setPosixFilePermissions(target, PosixFilePermissions.fromString("rw-------")); + final Path link = tempDir.resolve("link.txt"); + Files.createSymbolicLink(link, target); + + try { + FileUtils.defineFilePosixAttributeView(link, PosixFilePermissions.fromString("rw-rw-rw-"), null, null); + } catch (final IOException expected) { + // POSIX has no way to change the permissions of the link itself + } + + assertEquals( + "rw-------", + PosixFilePermissions.toString(Files.getPosixFilePermissions(target)), + "link target should have been left alone"); + } + @Nested class TestMkdir { @TempDir diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java index f5149bb7fa8..c44448078a8 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/PosixViewAttributeAction.java @@ -369,6 +369,10 @@ protected FileVisitor createFileVisitor(final Path basePath, final List() { @Override public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException { + if (attrs.isSymbolicLink()) { + LOGGER.trace("Not defining POSIX attribute on symbolic link {}", file); + return FileVisitResult.CONTINUE; + } for (final PathCondition pathFilter : conditions) { final Path relative = basePath.relativize(file); if (!pathFilter.accept(basePath, relative, attrs)) { diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java index 74ac05e64a2..159e1789269 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/FileUtils.java @@ -24,6 +24,7 @@ import java.nio.file.FileAlreadyExistsException; import java.nio.file.FileSystems; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.attribute.GroupPrincipal; import java.nio.file.attribute.PosixFileAttributeView; @@ -146,6 +147,10 @@ public static void makeParentDirs(final File file) throws IOException { /** * Define file POSIX attribute view on a path/file. + *

+ * Symbolic links are never followed: if {@code path} is a link, the attributes of the link itself are + * modified and its target is left untouched. + *

* * @param path Target path * @param filePermissions Permissions to apply @@ -159,7 +164,8 @@ public static void defineFilePosixAttributeView( final String fileOwner, final String fileGroup) throws IOException { - final PosixFileAttributeView view = Files.getFileAttributeView(path, PosixFileAttributeView.class); + final PosixFileAttributeView view = + Files.getFileAttributeView(path, PosixFileAttributeView.class, LinkOption.NOFOLLOW_LINKS); if (view != null) { final UserPrincipalLookupService lookupService = FileSystems.getDefault().getUserPrincipalLookupService(); diff --git a/src/changelog/.2.x.x/fix_posix_view_attribute_symlink.xml b/src/changelog/.2.x.x/fix_posix_view_attribute_symlink.xml new file mode 100644 index 00000000000..14aa8d43fb8 --- /dev/null +++ b/src/changelog/.2.x.x/fix_posix_view_attribute_symlink.xml @@ -0,0 +1,8 @@ + + + + Stop `PosixViewAttribute` from applying permissions and ownership through symbolic links +