From 57ad783bd5c59fcaf3e1b5d3c55bc5016dfe4186 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Wed, 21 Oct 2026 17:40:27 +0000 Subject: [PATCH 01/12] SOLR-18010: write standalone security.json through a temporary file, keep its permissions, drop internal metadata --- .../SOLR-18010-security-json-atomic-write.yml | 7 + .../admin/SecurityConfHandlerLocal.java | 48 ++- .../admin/SecurityConfHandlerTest.java | 294 ++++++++++++++++++ .../security/BasicAuthStandaloneTest.java | 25 ++ 4 files changed, 370 insertions(+), 4 deletions(-) create mode 100644 changelog/unreleased/SOLR-18010-security-json-atomic-write.yml diff --git a/changelog/unreleased/SOLR-18010-security-json-atomic-write.yml b/changelog/unreleased/SOLR-18010-security-json-atomic-write.yml new file mode 100644 index 000000000000..bc10eaa3779a --- /dev/null +++ b/changelog/unreleased/SOLR-18010-security-json-atomic-write.yml @@ -0,0 +1,7 @@ +title: Standalone security.json is written through a temporary file and moved into place, so a reader or a crash never sees a partly written document, existing file permissions are kept, and the file no longer carries internal version metadata. +type: fixed +authors: + - name: Nick Shanin +links: + - name: SOLR-18010 + url: https://issues.apache.org/jira/browse/SOLR-18010 diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index a5d951968001..572566bab251 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -19,11 +19,13 @@ import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; import java.lang.invoke.MethodHandles; +import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.List; +import java.util.Map; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.CommandOperation; import org.apache.solr.common.util.Utils; @@ -37,6 +39,7 @@ public class SecurityConfHandlerLocal extends SecurityConfHandler { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); protected Path securityJsonPath; + private final Object securityJsonLock = new Object(); public SecurityConfHandlerLocal(CoreContainer coreContainer) { @@ -96,15 +99,52 @@ protected boolean persistConf(SecurityConfig securityConfig) throws IOException SolrException.ErrorCode.SERVER_ERROR, "Failed persisting security.json to SOLR_HOME. Object was empty."); } - try (OutputStream securityJsonOs = Files.newOutputStream(securityJsonPath)) { - securityJsonOs.write(Utils.toJSON(securityConfig.getData())); - log.debug("Persisted security.json to {}", securityJsonPath); + @SuppressWarnings("unchecked") + Map sanitizedData = + (Map) Utils.getDeepCopy(securityConfig.getData(), 4); + for (Object pluginConfig : sanitizedData.values()) { + if (pluginConfig instanceof Map pluginConfigMap) { + pluginConfigMap.remove(""); + } + } + // Serialize first, then write to a temporary file in the same directory and move it over + // security.json. The file on disk is therefore always one complete document: a reader + // never observes a partially written file, and a crash mid-write leaves the previous + // document in place. + Path tmpPath = + Files.createTempFile( + securityJsonPath.getParent(), securityJsonPath.getFileName().toString(), ".tmp"); + try { + Files.write(tmpPath, Utils.toJSON(sanitizedData)); + if (Files.exists(securityJsonPath)) { + try { + Files.setPosixFilePermissions(tmpPath, Files.getPosixFilePermissions(securityJsonPath)); + } catch (UnsupportedOperationException | IOException e) { + if (log.isDebugEnabled()) { + log.debug("Could not keep the permissions of {}", securityJsonPath, e); + } + } + } + try { + Files.move( + tmpPath, + securityJsonPath, + StandardCopyOption.ATOMIC_MOVE, + StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(tmpPath, securityJsonPath, StandardCopyOption.REPLACE_EXISTING); + } + if (log.isDebugEnabled()) { + log.debug("Persisted security.json to {}", securityJsonPath); + } return true; } catch (Exception e) { throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, "Failed persisting security.json to " + securityJsonPath, e); + } finally { + Files.deleteIfExists(tmpPath); } } diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java index 5b5f4556c17e..a4b8df015731 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java @@ -17,11 +17,26 @@ package org.apache.solr.handler.admin; import static org.apache.solr.handler.admin.SecurityConfHandler.SecurityConfig; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.PosixFilePermission; +import java.util.Arrays; +import java.util.EnumSet; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.common.SolrErrorWrappingException; @@ -30,6 +45,7 @@ import org.apache.solr.common.util.CommandOperation; import org.apache.solr.common.util.ContentStreamBase; import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; import org.apache.solr.request.SolrQueryRequestBase; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.security.BasicAuthPlugin; @@ -178,6 +194,284 @@ public void testEdit() throws Exception { handler.close(); } + /** + * Two edits to the standalone security.json that overlap in time must both survive: each edit + * reads the file, applies its commands and rewrites the file, so an edit that starts from the + * same content as another one overwrites it. The first edit is held open inside the plugin while + * the second edit is issued, which is the interleave a caller produces by sending two edit + * requests without waiting for the first response (SOLR-18010). + */ + @SuppressWarnings({"unchecked"}) + public void testConcurrentEditsToLocalSecurityJson() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocal"); + Path securityJson = solrHome.resolve("security.json"); + String seed = + "{\"authentication\":{\"class\":\"solr.BasicAuthPlugin\"," + + "\"credentials\":{\"solr\":\"solr\"}}," + + "\"authorization\":{\"class\":\"solr.RuleBasedAuthorizationPlugin\",\"user-role\":{}}}"; + Files.write(securityJson, seed.getBytes(StandardCharsets.UTF_8)); + + LatchingAuthorizationPlugin authzPlugin = new LatchingAuthorizationPlugin(); + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + when(cc.getAuthorizationPlugin()).thenReturn(authzPlugin); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(cc); + + AtomicReference firstErr = new AtomicReference<>(); + AtomicReference secondErr = new AtomicReference<>(); + Thread first = + new Thread( + () -> { + try { + postAuthorizationEdit(handler, "{'set-user-role': {'alice': 'admin'}}"); + } catch (Throwable t) { + firstErr.set(t); + } + }); + first.start(); + assertTrue( + "first edit reached the plugin", authzPlugin.editEntered.await(60, TimeUnit.SECONDS)); + Thread second = + new Thread( + () -> { + try { + postAuthorizationEdit(handler, "{'set-user-role': {'bob': 'dev'}}"); + } catch (Throwable t) { + secondErr.set(t); + } + }); + second.start(); + // If edits are not serialized, the second edit now runs to completion against the same + // starting content the first edit read. If they are serialized it is still waiting, and + // the join simply times out. + second.join(TimeUnit.SECONDS.toMillis(10)); + authzPlugin.editRelease.countDown(); + first.join(TimeUnit.SECONDS.toMillis(60)); + second.join(TimeUnit.SECONDS.toMillis(60)); + assertFalse("first edit still running", first.isAlive()); + assertFalse("second edit still running", second.isAlive()); + assertNull("first edit failed: " + firstErr.get(), firstErr.get()); + assertNull("second edit failed: " + secondErr.get(), secondErr.get()); + + byte[] fileBytes = Files.readAllBytes(securityJson); + assertSingleJsonDocument(fileBytes); + Map data = (Map) Utils.fromJSON(fileBytes); + Map userRoles = + (Map) ((Map) data.get("authorization")).get("user-role"); + assertEquals("alice's role assignment was lost", "admin", userRoles.get("alice")); + assertEquals("bob's role assignment was lost", "dev", userRoles.get("bob")); + } + + /** + * Two overlapping writes of security.json must never leave a torn file behind: the document on + * disk afterwards is always exactly one of the written documents, never the beginning of one + * followed by the tail of a longer one (the corruption shape in SOLR-18010). The first write is + * held open inside serialization while the second write runs, which is the interleave that + * produced that shape. + */ + public void testConcurrentPersistConfLeavesOneWholeDocument() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalPersist"); + Path securityJson = solrHome.resolve("security.json"); + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + SecurityConfHandlerLocalForTesting handler = new SecurityConfHandlerLocalForTesting(cc); + + byte[] shortBytes = Utils.toJSON(shortSecurityData("x")); + byte[] longBytes = Utils.toJSON(longSecurityData()); + Files.write(securityJson, Utils.toJSON(shortSecurityData("seed"))); + + CountDownLatch entered = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + AtomicReference firstErr = new AtomicReference<>(); + Thread first = + new Thread( + () -> { + try { + handler.persistConf( + new SecurityConfig() + .setData(shortSecurityData(new LatchingValue("x", entered, release)))); + } catch (Throwable t) { + firstErr.set(t); + } + }); + first.start(); + assertTrue("first write reached serialization", entered.await(60, TimeUnit.SECONDS)); + handler.persistConf(new SecurityConfig().setData(longSecurityData())); + release.countDown(); + first.join(TimeUnit.SECONDS.toMillis(60)); + assertFalse("first write still running", first.isAlive()); + assertNull("first write failed: " + firstErr.get(), firstErr.get()); + + byte[] fileBytes = Files.readAllBytes(securityJson); + assertSingleJsonDocument(fileBytes); + assertTrue( + "security.json holds neither written document (" + + fileBytes.length + + " bytes; the writes were " + + shortBytes.length + + " and " + + longBytes.length + + " bytes)", + Arrays.equals(fileBytes, shortBytes) || Arrays.equals(fileBytes, longBytes)); + } + + /** An edit to security.json keeps the POSIX permissions of the existing file. */ + public void testEditKeepsExistingSecurityJsonPermissions() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalPerms"); + assumeTrue( + "POSIX file permissions are not supported on this file system", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path securityJson = solrHome.resolve("security.json"); + String seed = + "{\"authentication\":{\"class\":\"solr.BasicAuthPlugin\"," + + "\"credentials\":{\"solr\":\"solr\"}}," + + "\"authorization\":{\"class\":\"solr.RuleBasedAuthorizationPlugin\",\"user-role\":{}}}"; + Files.write(securityJson, seed.getBytes(StandardCharsets.UTF_8)); + EnumSet expected = + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.GROUP_READ); + Files.setPosixFilePermissions(securityJson, expected); + + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + when(cc.getAuthorizationPlugin()).thenReturn(new RuleBasedAuthorizationPlugin()); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(cc); + postAuthorizationEdit(handler, "{'set-user-role': {'alice': 'admin'}}"); + + assertTrue( + "edit was not persisted", + Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertEquals( + "security.json lost its permissions in the edit", + expected, + Files.getPosixFilePermissions(securityJson)); + } + + private static void postAuthorizationEdit(SecurityConfHandler handler, String command) + throws Exception { + SolrQueryRequestBase req = new SolrQueryRequestBase(null, new ModifiableSolrParams()); + req.getContext().put("httpMethod", SolrRequest.METHOD.POST); + req.getContext().put("path", "/admin/authorization"); + req.setContentStreams( + List.of( + new ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), ""))); + handler.handleRequestBody(req, new SolrQueryResponse()); + } + + private static void assertSingleJsonDocument(byte[] fileBytes) { + ObjectMapper mapper = + JsonMapper.builder().enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS).build(); + try { + mapper.readValue(fileBytes, Map.class); + } catch (Exception e) { + fail("security.json is not a single JSON document: " + e); + } + } + + private static Map shortSecurityData(CharSequence credValue) { + Map creds = new LinkedHashMap<>(); + creds.put("u", credValue); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + private static Map longSecurityData() { + StringBuilder pad = new StringBuilder(); + for (int i = 0; i < 6000; i++) { + pad.append('L'); + } + Map creds = new LinkedHashMap<>(); + creds.put("longuser", pad.toString()); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + /** An authorization plugin whose first {@code edit} call blocks until the test releases it. */ + private static class LatchingAuthorizationPlugin extends RuleBasedAuthorizationPlugin { + final CountDownLatch editEntered = new CountDownLatch(1); + final CountDownLatch editRelease = new CountDownLatch(1); + private final AtomicBoolean armed = new AtomicBoolean(true); + + @Override + public Map edit( + Map latestConf, List commands) { + if (armed.compareAndSet(true, false)) { + editEntered.countDown(); + try { + editRelease.await(60, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + return super.edit(latestConf, commands); + } + } + + /** + * A {@link CharSequence} whose first read blocks until released. Serialization evaluates it, so a + * write whose data carries one is held open at a known point inside the write. + */ + private static final class LatchingValue implements CharSequence { + private final String delegate; + private final CountDownLatch entered; + private final CountDownLatch release; + private final AtomicBoolean armed = new AtomicBoolean(true); + + LatchingValue(String delegate, CountDownLatch entered, CountDownLatch release) { + this.delegate = delegate; + this.entered = entered; + this.release = release; + } + + private void gate() { + if (armed.compareAndSet(true, false)) { + entered.countDown(); + try { + release.await(60, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + } + + @Override + public int length() { + gate(); + return delegate.length(); + } + + @Override + public char charAt(int index) { + gate(); + return delegate.charAt(index); + } + + @Override + public CharSequence subSequence(int start, int end) { + gate(); + return delegate.subSequence(start, end); + } + + @Override + public String toString() { + gate(); + return delegate; + } + } + public static class MockSecurityHandler extends SecurityConfHandler { private Map m; final BasicAuthPlugin basicAuthPlugin = new BasicAuthPlugin(); diff --git a/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java b/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java index 35ff50dd0507..868d4d146de1 100644 --- a/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java +++ b/solr/core/src/test/org/apache/solr/security/BasicAuthStandaloneTest.java @@ -22,6 +22,7 @@ import static org.apache.solr.security.BasicAuthIntegrationTest.verifySecurityStatus; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Base64; @@ -111,6 +112,25 @@ public void testBasicAuth() throws Exception { new String(Utils.toJSON(securityConfHandler.getSecurityConfig(false).getData()), UTF_8) .contains("harry")); + command = "{\n" + "'set-user-role': {'harry':'admin'}\n" + "}"; + doHttpPost(httpClient, baseUrl + authzPrefix, command, "solr", "SolrRocks"); + verifySecurityStatus( + httpClient, baseUrl + authzPrefix, "authorization/user-role/harry", "admin", 20); + + Map persistedSecurityJson = + asMap( + Utils.fromJSONString( + Files.readString(instance.getHomeDir().resolve("security.json"), UTF_8))); + Map persistedAuthc = asMap(persistedSecurityJson.get("authentication")); + Map persistedAuthz = asMap(persistedSecurityJson.get("authorization")); + assertFalse( + "authentication config should not contain internal metadata", + persistedAuthc.containsKey("")); + assertFalse( + "authorization config should not contain internal metadata", + persistedAuthz.containsKey("")); + assertEquals("admin", ((Map) persistedAuthz.get("user-role")).get("harry")); + // Edit authorization verifySecurityStatus( httpClient, baseUrl + authzPrefix, "authorization/permissions[1]/role", null, 20); @@ -226,6 +246,11 @@ static void doHttpPostWithHeader( } } + @SuppressWarnings("unchecked") + private static Map asMap(Object value) { + return (Map) value; + } + private static String encodeBasicAuthHeaderIfNotNull(String user, String pwd) { if (user == null && pwd == null) { return null; From dfcf50aec035450f33982859b2125b3587049f71 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Wed, 21 Oct 2026 18:42:20 +0000 Subject: [PATCH 02/12] SOLR-18010: keep the temporary-file cleanup from masking a persist failure --- .../solr/handler/admin/SecurityConfHandlerLocal.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index 572566bab251..163244a91b78 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -39,7 +39,6 @@ public class SecurityConfHandlerLocal extends SecurityConfHandler { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); protected Path securityJsonPath; - private final Object securityJsonLock = new Object(); public SecurityConfHandlerLocal(CoreContainer coreContainer) { @@ -144,7 +143,11 @@ protected boolean persistConf(SecurityConfig securityConfig) throws IOException "Failed persisting security.json to " + securityJsonPath, e); } finally { - Files.deleteIfExists(tmpPath); + try { + Files.deleteIfExists(tmpPath); + } catch (IOException e) { + log.warn("Could not remove temporary file {}", tmpPath, e); + } } } From 09c8e5c8f6b5e6cef58eaaaed16ee8e22b794e08 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Wed, 21 Oct 2026 20:22:46 +0000 Subject: [PATCH 03/12] SOLR-18010: pin an edit in a read-only SOLR_HOME --- .../admin/SecurityConfHandlerTest.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java index a4b8df015731..6da4708a5df1 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java @@ -352,6 +352,43 @@ public void testEditKeepsExistingSecurityJsonPermissions() throws Exception { Files.getPosixFilePermissions(securityJson)); } + /** An edit in a read-only SOLR_HOME still succeeds when security.json itself is writable. */ + public void testEditInPlaceWhenSolrHomeIsReadOnly() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalReadOnly"); + assumeTrue( + "POSIX file permissions are not supported on this file system", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path securityJson = solrHome.resolve("security.json"); + String seed = + "{\"authentication\":{\"class\":\"solr.BasicAuthPlugin\"," + + "\"credentials\":{\"solr\":\"solr\"}}," + + "\"authorization\":{\"class\":\"solr.RuleBasedAuthorizationPlugin\",\"user-role\":{}}}"; + Files.write(securityJson, seed.getBytes(StandardCharsets.UTF_8)); + Files.setPosixFilePermissions( + solrHome, EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_EXECUTE)); + try { + assumeFalse("the directory is writable for this user", Files.isWritable(solrHome)); + + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + when(cc.getAuthorizationPlugin()).thenReturn(new RuleBasedAuthorizationPlugin()); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(cc); + postAuthorizationEdit(handler, "{'set-user-role': {'alice': 'admin'}}"); + + assertTrue( + "edit was not persisted", + Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + } finally { + Files.setPosixFilePermissions( + solrHome, + EnumSet.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE)); + } + } + private static void postAuthorizationEdit(SecurityConfHandler handler, String command) throws Exception { SolrQueryRequestBase req = new SolrQueryRequestBase(null, new ModifiableSolrParams()); From 1b14ce9556fa1357f41ca0aefebb435a97ef1504 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Wed, 21 Oct 2026 20:36:58 +0000 Subject: [PATCH 04/12] SOLR-18010: write security.json in place when the temporary file cannot replace it --- .../SOLR-18010-security-json-atomic-write.yml | 2 +- .../admin/SecurityConfHandlerLocal.java | 59 +++++++++++++------ 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/changelog/unreleased/SOLR-18010-security-json-atomic-write.yml b/changelog/unreleased/SOLR-18010-security-json-atomic-write.yml index bc10eaa3779a..ad37016d9623 100644 --- a/changelog/unreleased/SOLR-18010-security-json-atomic-write.yml +++ b/changelog/unreleased/SOLR-18010-security-json-atomic-write.yml @@ -1,4 +1,4 @@ -title: Standalone security.json is written through a temporary file and moved into place, so a reader or a crash never sees a partly written document, existing file permissions are kept, and the file no longer carries internal version metadata. +title: Standalone security.json is replaced through a temporary file, so a process crash during an edit no longer leaves a partly written file; the file keeps its permissions and no longer stores internal version metadata. Symbolic links and directories that cannot take a new file are still written in place. type: fixed authors: - name: Nick Shanin diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index 163244a91b78..40c98b5b38a8 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -106,22 +106,52 @@ protected boolean persistConf(SecurityConfig securityConfig) throws IOException pluginConfigMap.remove(""); } } - // Serialize first, then write to a temporary file in the same directory and move it over - // security.json. The file on disk is therefore always one complete document: a reader - // never observes a partially written file, and a crash mid-write leaves the previous - // document in place. + try { + replaceSecurityJson(Utils.toJSON(sanitizedData)); + if (log.isDebugEnabled()) { + log.debug("Persisted security.json to {}", securityJsonPath); + } + return true; + } catch (Exception e) { + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, + "Failed persisting security.json to " + securityJsonPath, + e); + } + } + + /** + * Replaces security.json through a temporary file in the same directory. A symbolic link, a file + * whose owner or permissions cannot carry over, and a file the directory cannot replace are + * written in place. + */ + private void replaceSecurityJson(byte[] json) throws IOException { + if (!Files.isSymbolicLink(securityJsonPath)) { + try { + replaceThroughTemporaryFile(json); + return; + } catch (IOException e) { + log.warn( + "Could not replace {} through a temporary file, writing it in place", + securityJsonPath, + e); + } + } + Files.write(securityJsonPath, json); + } + + private void replaceThroughTemporaryFile(byte[] json) throws IOException { Path tmpPath = Files.createTempFile( securityJsonPath.getParent(), securityJsonPath.getFileName().toString(), ".tmp"); try { - Files.write(tmpPath, Utils.toJSON(sanitizedData)); + Files.write(tmpPath, json); if (Files.exists(securityJsonPath)) { - try { + if (tmpPath.getFileSystem().supportedFileAttributeViews().contains("posix")) { Files.setPosixFilePermissions(tmpPath, Files.getPosixFilePermissions(securityJsonPath)); - } catch (UnsupportedOperationException | IOException e) { - if (log.isDebugEnabled()) { - log.debug("Could not keep the permissions of {}", securityJsonPath, e); - } + } + if (!Files.getOwner(tmpPath).equals(Files.getOwner(securityJsonPath))) { + throw new IOException("security.json is owned by " + Files.getOwner(securityJsonPath)); } } try { @@ -133,15 +163,6 @@ protected boolean persistConf(SecurityConfig securityConfig) throws IOException } catch (AtomicMoveNotSupportedException e) { Files.move(tmpPath, securityJsonPath, StandardCopyOption.REPLACE_EXISTING); } - if (log.isDebugEnabled()) { - log.debug("Persisted security.json to {}", securityJsonPath); - } - return true; - } catch (Exception e) { - throw new SolrException( - SolrException.ErrorCode.SERVER_ERROR, - "Failed persisting security.json to " + securityJsonPath, - e); } finally { try { Files.deleteIfExists(tmpPath); From d1bf9b140729c4b196781d65c0f7c8ca0f678c27 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Thu, 22 Oct 2026 19:01:58 +0000 Subject: [PATCH 05/12] SOLR-18010: Keep security.json's group and fail edits on temporary file errors Only a SOLR_HOME that denies writes falls back to an in-place write when the temporary file cannot be created. Other failures, such as running out of inodes, fail the edit and keep the existing file, since an in-place write would truncate it first. The replacement file takes the group of the existing security.json, the same as it already keeps the owner and mode. If the group cannot be set, the edit is written in place, as for a file owned by another user. --- ...OLR-18010-security-json-atomic-replace.yml | 2 +- .../admin/SecurityConfHandlerLocal.java | 21 +++++++++++++++++-- .../admin/SecurityConfHandlerTest.java | 3 +++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml b/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml index 17624adeeb70..90b200cee8a0 100644 --- a/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml +++ b/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml @@ -1,4 +1,4 @@ -title: Standalone security.json is replaced through a synced temporary file on POSIX file systems, so a crash during an edit leaves the previous file in place. Symbolic links and other file systems are written in place as before. +title: Standalone security.json is replaced through a synced temporary file on POSIX file systems, so a process crash during an edit leaves the previous file in place. Symbolic links and other file systems are written in place as before. type: fixed authors: - name: Nick Shanin diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index 07e287b21ea1..5b59a2f368b2 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -22,11 +22,15 @@ import java.io.OutputStream; import java.lang.invoke.MethodHandles; import java.nio.channels.FileChannel; +import java.nio.file.AccessDeniedException; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFileAttributes; import java.util.List; import java.util.Set; import java.util.UUID; @@ -124,7 +128,7 @@ private void replaceSecurityJson(byte[] json) throws IOException { securityJsonPath.getFileName() + "." + UUID.randomUUID() + ".tmp"); try { Files.createFile(tmpPath); - } catch (IOException e) { + } catch (AccessDeniedException e) { log.warn( "Could not create a temporary file next to {}, writing it in place", securityJsonPath, e); writeInPlace(json); @@ -134,12 +138,19 @@ private void replaceSecurityJson(byte[] json) throws IOException { Files.write(tmpPath, json); syncFile(tmpPath); if (Files.exists(securityJsonPath)) { - copyPermissions(tmpPath); if (!hasSameOwner(tmpPath)) { log.warn("{} is owned by another user, writing it in place", securityJsonPath); writeInPlace(json); return; } + try { + copyGroup(tmpPath); + } catch (IOException e) { + log.warn("Could not keep the group of {}, writing it in place", securityJsonPath, e); + writeInPlace(json); + return; + } + copyPermissions(tmpPath); } try { moveTemporaryFile(tmpPath); @@ -196,6 +207,12 @@ private void copyPermissions(Path tmpPath) throws IOException { Files.setPosixFilePermissions(tmpPath, Files.getPosixFilePermissions(securityJsonPath)); } + private void copyGroup(Path tmpPath) throws IOException { + GroupPrincipal group = + Files.readAttributes(securityJsonPath, PosixFileAttributes.class).group(); + Files.getFileAttributeView(tmpPath, PosixFileAttributeView.class).setGroup(group); + } + private boolean hasSameOwner(Path tmpPath) throws IOException { return Files.getOwner(tmpPath).equals(Files.getOwner(securityJsonPath)); } diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java index 27e0d0c8e694..2116c91c3db9 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java @@ -329,16 +329,19 @@ public void testReaderNeverSeesPartialSecurityJsonDuringEdits() throws Exception } }); writer.start(); + int reads = 0; try { while (writing.get()) { byte[] got = Files.readAllBytes(securityJson); assertTrue( "reader saw a partial security.json of " + got.length + " bytes", Arrays.equals(got, smallBytes) || Arrays.equals(got, largeBytes)); + reads++; } } finally { writer.join(TimeUnit.SECONDS.toMillis(60)); } + assertTrue("reader made no reads while the writer ran", reads > 0); assertNull("writer failed: " + writerErr.get(), writerErr.get()); } From e7d1c336f470d3f3dbd97cd9ddb75b9a045a4f99 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Thu, 22 Oct 2026 20:30:13 +0000 Subject: [PATCH 06/12] SOLR-18010: run the security.json handler tests without the Windows-like test filesystem The reader test renames over a file it holds open. The Windows-like test filesystem tracks open files by inode and fails its bookkeeping assertion in that case (seed 75F671E94C57A51). The class runs without that filesystem. On NFS a read can fail with a stale handle while the file is replaced; the reader skips those reads and still checks every successful one. --- .../solr/handler/admin/SecurityConfHandlerTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java index 2116c91c3db9..3c260d22ba10 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java @@ -40,6 +40,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; +import org.apache.lucene.tests.util.LuceneTestCase.SuppressFileSystems; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.common.SolrErrorWrappingException; @@ -54,6 +55,7 @@ import org.apache.solr.security.BasicAuthPlugin; import org.apache.solr.security.RuleBasedAuthorizationPlugin; +@SuppressFileSystems({"ExtrasFS", "WindowsFS"}) public class SecurityConfHandlerTest extends SolrTestCaseJ4 { @SuppressWarnings({"unchecked", "rawtypes"}) @@ -332,7 +334,13 @@ public void testReaderNeverSeesPartialSecurityJsonDuringEdits() throws Exception int reads = 0; try { while (writing.get()) { - byte[] got = Files.readAllBytes(securityJson); + byte[] got; + try { + got = Files.readAllBytes(securityJson); + } catch (IOException e) { + // NFS reports a stale handle when the file is replaced during a read + continue; + } assertTrue( "reader saw a partial security.json of " + got.length + " bytes", Arrays.equals(got, smallBytes) || Arrays.equals(got, largeBytes)); From 55962f9d344f25af729f7079eb3ee4ca0a0bb9f0 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Thu, 22 Oct 2026 22:35:26 +0000 Subject: [PATCH 07/12] SOLR-18010: write a hard-linked security.json in place and move the reader test to its own class Renaming over a hard-linked file breaks the link, so a second name for the same file kept the old content. Such files are written in place, as before. The reader test replaces a file while it is open, which the Windows-like test filesystem cannot track, so it moves to SecurityConfHandlerReaderTest, which runs without that filesystem. --- ...OLR-18010-security-json-atomic-replace.yml | 2 +- .../admin/SecurityConfHandlerLocal.java | 12 +- .../admin/SecurityConfHandlerReaderTest.java | 126 ++++++++++++++++++ .../admin/SecurityConfHandlerTest.java | 85 +++--------- ...hentication-and-authorization-plugins.adoc | 4 + 5 files changed, 160 insertions(+), 69 deletions(-) create mode 100644 solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java diff --git a/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml b/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml index 90b200cee8a0..e2786e61dec8 100644 --- a/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml +++ b/changelog/unreleased/SOLR-18010-security-json-atomic-replace.yml @@ -1,4 +1,4 @@ -title: Standalone security.json is replaced through a synced temporary file on POSIX file systems, so a process crash during an edit leaves the previous file in place. Symbolic links and other file systems are written in place as before. +title: Standalone security.json is replaced through a synced temporary file on POSIX file systems, so a process crash during an edit leaves the previous file in place. Symbolic links, hard-linked files and file systems without POSIX attributes are written in place as before. type: fixed authors: - name: Nick Shanin diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index 5b59a2f368b2..bd9f98585081 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -176,10 +176,16 @@ private void writeInPlace(byte[] json) throws IOException { } } - private boolean atomicReplaceSupported() { + private boolean atomicReplaceSupported() throws IOException { return supportsAtomicReplace( - securityJsonPath.getFileSystem().supportedFileAttributeViews(), - Files.isSymbolicLink(securityJsonPath)); + securityJsonPath.getFileSystem().supportedFileAttributeViews(), + Files.isSymbolicLink(securityJsonPath)) + && !hasHardLinks(); + } + + private boolean hasHardLinks() throws IOException { + return Files.exists(securityJsonPath) + && (int) Files.getAttribute(securityJsonPath, "unix:nlink") > 1; } static boolean supportsAtomicReplace(Set fileAttributeViews, boolean symbolicLink) { diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java new file mode 100644 index 000000000000..7606cb839bb5 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java @@ -0,0 +1,126 @@ +/* + * 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.solr.handler.admin; + +import static org.apache.solr.handler.admin.SecurityConfHandler.SecurityConfig; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.lucene.tests.util.LuceneTestCase.SuppressFileSystems; +import org.apache.solr.SolrTestCase; +import org.apache.solr.common.util.Utils; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.security.RuleBasedAuthorizationPlugin; + +/** + * Reads security.json while edits replace it. The reader keeps the file open while the writer + * renames a new file over it. The Windows-like test filesystem cannot track that case, so this + * class runs without it. + */ +@SuppressFileSystems({"ExtrasFS", "WindowsFS"}) +public class SecurityConfHandlerReaderTest extends SolrTestCase { + + /** A reader of security.json never sees a partial document while edits are written. */ + public void testReaderNeverSeesPartialSecurityJsonDuringEdits() throws Exception { + Path solrHome = createTempDir("securityConfHandlerLocalReaders"); + assumeTrue( + "atomic replacement needs POSIX file attributes", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path securityJson = solrHome.resolve("security.json"); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + Map small = shortSecurityData("x"); + Map large = largeSecurityData(); + byte[] smallBytes = Utils.toJSON(small); + byte[] largeBytes = Utils.toJSON(large); + Files.write(securityJson, smallBytes); + AtomicReference writerErr = new AtomicReference<>(); + AtomicBoolean writing = new AtomicBoolean(true); + Thread writer = + new Thread( + () -> { + try { + for (int i = 0; i < 100; i++) { + handler.persistConf(new SecurityConfig().setData(i % 2 == 0 ? large : small)); + } + } catch (Throwable t) { + writerErr.set(t); + } finally { + writing.set(false); + } + }); + writer.start(); + int reads = 0; + try { + while (writing.get()) { + byte[] got; + try { + got = Files.readAllBytes(securityJson); + } catch (IOException e) { + // NFS reports a stale handle when the file is replaced during a read + continue; + } + assertTrue( + "reader saw a partial security.json of " + got.length + " bytes", + Arrays.equals(got, smallBytes) || Arrays.equals(got, largeBytes)); + reads++; + } + } finally { + writer.join(TimeUnit.SECONDS.toMillis(60)); + } + assertTrue("reader made no reads while the writer ran", reads > 0); + assertNull("writer failed: " + writerErr.get(), writerErr.get()); + } + + private static Map largeSecurityData() { + Map creds = new LinkedHashMap<>(); + creds.put("large", "L".repeat(1_000_000)); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + private static Map shortSecurityData(CharSequence credValue) { + Map creds = new LinkedHashMap<>(); + creds.put("u", credValue); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + + private static CoreContainer mockCoreContainer(Path solrHome) { + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + when(cc.getAuthorizationPlugin()).thenReturn(new RuleBasedAuthorizationPlugin()); + return cc; + } +} diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java index 3c260d22ba10..3d800dbdbe64 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java @@ -40,7 +40,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; -import org.apache.lucene.tests.util.LuceneTestCase.SuppressFileSystems; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.common.SolrErrorWrappingException; @@ -55,7 +54,6 @@ import org.apache.solr.security.BasicAuthPlugin; import org.apache.solr.security.RuleBasedAuthorizationPlugin; -@SuppressFileSystems({"ExtrasFS", "WindowsFS"}) public class SecurityConfHandlerTest extends SolrTestCaseJ4 { @SuppressWarnings({"unchecked", "rawtypes"}) @@ -275,6 +273,26 @@ public void testEditKeepsExistingSecurityJsonPermissions() throws Exception { assertNoTemporaryFiles(solrHome); } + /** An edit to a hard-linked security.json is written in place, so the other name sees it. */ + public void testEditKeepsHardLinkedSecurityJsonShared() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalHardLink"); + Path securityJson = solrHome.resolve("security.json"); + Path other = solrHome.resolve("other.json"); + Files.write(securityJson, seedSecurityJson().getBytes(StandardCharsets.UTF_8)); + boolean linked = true; + try { + Files.createLink(other, securityJson); + } catch (IOException | UnsupportedOperationException e) { + linked = false; + } + assumeTrue("hard links are not supported on this file system", linked); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, "{'set-user-role': {'alice': 'admin'}}"); + assertTrue(Files.readString(other, StandardCharsets.UTF_8).contains("alice")); + assertNoTemporaryFiles(solrHome); + } + /** A read-only SOLR_HOME still allows an edit when security.json itself is writable. */ public void testEditInPlaceWhenSolrHomeIsReadOnly() throws Exception { assumeWorkingMockito(); @@ -301,69 +319,6 @@ public void testEditInPlaceWhenSolrHomeIsReadOnly() throws Exception { } } - /** A reader of security.json never sees a partial document while edits are written. */ - public void testReaderNeverSeesPartialSecurityJsonDuringEdits() throws Exception { - assumeWorkingMockito(); - Path solrHome = createTempDir("securityConfHandlerLocalReaders"); - assumeTrue( - "atomic replacement needs POSIX file attributes", - solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); - Path securityJson = solrHome.resolve("security.json"); - SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); - Map small = shortSecurityData("x"); - Map large = largeSecurityData(); - byte[] smallBytes = Utils.toJSON(small); - byte[] largeBytes = Utils.toJSON(large); - Files.write(securityJson, smallBytes); - AtomicReference writerErr = new AtomicReference<>(); - AtomicBoolean writing = new AtomicBoolean(true); - Thread writer = - new Thread( - () -> { - try { - for (int i = 0; i < 100; i++) { - handler.persistConf(new SecurityConfig().setData(i % 2 == 0 ? large : small)); - } - } catch (Throwable t) { - writerErr.set(t); - } finally { - writing.set(false); - } - }); - writer.start(); - int reads = 0; - try { - while (writing.get()) { - byte[] got; - try { - got = Files.readAllBytes(securityJson); - } catch (IOException e) { - // NFS reports a stale handle when the file is replaced during a read - continue; - } - assertTrue( - "reader saw a partial security.json of " + got.length + " bytes", - Arrays.equals(got, smallBytes) || Arrays.equals(got, largeBytes)); - reads++; - } - } finally { - writer.join(TimeUnit.SECONDS.toMillis(60)); - } - assertTrue("reader made no reads while the writer ran", reads > 0); - assertNull("writer failed: " + writerErr.get(), writerErr.get()); - } - - private static Map largeSecurityData() { - Map creds = new LinkedHashMap<>(); - creds.put("large", "L".repeat(1_000_000)); - Map authc = new LinkedHashMap<>(); - authc.put("class", "solr.BasicAuthPlugin"); - authc.put("credentials", creds); - Map data = new LinkedHashMap<>(); - data.put("authentication", authc); - return data; - } - /** A security.json created by a write gets the mode a newly created file gets. */ public void testPersistCreatesSecurityJsonWithDefaultMode() throws Exception { assumeWorkingMockito(); diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/authentication-and-authorization-plugins.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/authentication-and-authorization-plugins.adoc index 61abd3fc8a75..8fadfbe2e8bb 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/authentication-and-authorization-plugins.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/authentication-and-authorization-plugins.adoc @@ -119,6 +119,10 @@ With a user-managed cluster, you will need to place `security.json` on each node You can use the authentication and authorization APIs, but with a user-managed cluster you will need to make the same API requests on each node separately. You can also edit `security.json` by hand if you prefer. +In standalone mode on a POSIX file system, Solr saves `security.json` by writing a new file and renaming it over the old one, so a reader never sees a partial file. +The new file does not keep POSIX ACLs or SELinux labels from the old file, so set them again after an edit. +A hard-linked `security.json` is written in place instead, so its links keep sharing the same content. + [#configuring-authentication] == Authentication From c34abaec94fe795dee5fcbeffaa88c0981c25f92 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Thu, 22 Oct 2026 23:04:25 +0000 Subject: [PATCH 08/12] SOLR-18010: call assumeWorkingMockito in the reader test The module check requires the call in any test file that uses Mockito. --- .../solr/handler/admin/SecurityConfHandlerReaderTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java index 7606cb839bb5..f11d8a026cd9 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerReaderTest.java @@ -17,6 +17,7 @@ package org.apache.solr.handler.admin; +import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito; import static org.apache.solr.handler.admin.SecurityConfHandler.SecurityConfig; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -46,6 +47,7 @@ public class SecurityConfHandlerReaderTest extends SolrTestCase { /** A reader of security.json never sees a partial document while edits are written. */ public void testReaderNeverSeesPartialSecurityJsonDuringEdits() throws Exception { + assumeWorkingMockito(); Path solrHome = createTempDir("securityConfHandlerLocalReaders"); assumeTrue( "atomic replacement needs POSIX file attributes", From 50d1e1afcc041c1e9be6badc2392ee9b538f4398 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Thu, 22 Oct 2026 23:59:12 +0000 Subject: [PATCH 09/12] SOLR-18010: check that the temporary file is no wider than security.json --- .../admin/SecurityConfHandlerTest.java | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java index 3d800dbdbe64..19e3c783a843 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerTest.java @@ -27,6 +27,9 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermission; import java.util.Arrays; import java.util.EnumSet; @@ -35,6 +38,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -266,10 +270,64 @@ public void testEditKeepsExistingSecurityJsonPermissions() throws Exception { PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_READ); Files.setPosixFilePermissions(securityJson, expected); + Object fileKey = Files.readAttributes(securityJson, BasicFileAttributes.class).fileKey(); + GroupPrincipal group = Files.readAttributes(securityJson, PosixFileAttributes.class).group(); SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); postAuthorizationEdit(handler, "{'set-user-role': {'alice': 'admin'}}"); assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); assertEquals(expected, Files.getPosixFilePermissions(securityJson)); + assertEquals(group, Files.readAttributes(securityJson, PosixFileAttributes.class).group()); + assertNotEquals( + "security.json was written in place, not replaced", + fileKey, + Files.readAttributes(securityJson, BasicFileAttributes.class).fileKey()); + assertNoTemporaryFiles(solrHome); + } + + /** While an edit is written, the temporary file is no wider than the existing security.json. */ + public void testTemporaryFileIsNoWiderThanExistingSecurityJson() throws Exception { + assumeWorkingMockito(); + Path solrHome = createTempDir("securityConfHandlerLocalTmpMode"); + assumeTrue( + "POSIX file permissions are not supported on this file system", + solrHome.getFileSystem().supportedFileAttributeViews().contains("posix")); + Path securityJson = solrHome.resolve("security.json"); + Files.write(securityJson, seedSecurityJson().getBytes(StandardCharsets.UTF_8)); + Set existing = + EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE); + Files.setPosixFilePermissions(securityJson, existing); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + Map bulk = bulkSecurityData(); + Set> seen = ConcurrentHashMap.newKeySet(); + AtomicBoolean writing = new AtomicBoolean(true); + Thread watcher = + new Thread( + () -> { + while (writing.get()) { + try (Stream files = Files.list(solrHome)) { + for (Path file : + files.filter(f -> f.getFileName().toString().endsWith(".tmp")).toList()) { + seen.add(Files.getPosixFilePermissions(file)); + } + } catch (IOException e) { + // the temporary file was renamed or deleted between the listing and the read + } + } + }); + watcher.start(); + try { + for (int i = 0; i < 3; i++) { + handler.persistConf(new SecurityConfig().setData(bulk)); + } + } finally { + writing.set(false); + watcher.join(TimeUnit.SECONDS.toMillis(60)); + } + assertFalse("the watcher never saw a temporary file", seen.isEmpty()); + for (Set mode : seen) { + assertTrue( + "temporary file mode " + mode + " is wider than " + existing, existing.containsAll(mode)); + } assertNoTemporaryFiles(solrHome); } @@ -417,6 +475,17 @@ private static Map longSecurityData() { return data; } + private static Map bulkSecurityData() { + Map creds = new LinkedHashMap<>(); + creds.put("bulk", "B".repeat(8_000_000)); + Map authc = new LinkedHashMap<>(); + authc.put("class", "solr.BasicAuthPlugin"); + authc.put("credentials", creds); + Map data = new LinkedHashMap<>(); + data.put("authentication", authc); + return data; + } + /** * A {@link CharSequence} whose first read blocks until released. Serialization evaluates it, so a * write whose data carries one is held open at a known point inside the write. From d8388c6519668102a8d37be1627f81e7f3ef4f70 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Thu, 22 Oct 2026 23:59:13 +0000 Subject: [PATCH 10/12] SOLR-18010: create the temporary file owner-only while replacing security.json --- .../handler/admin/SecurityConfHandlerLocal.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java index bd9f98585081..62f850dc18a7 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/SecurityConfHandlerLocal.java @@ -28,9 +28,13 @@ import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.GroupPrincipal; import java.nio.file.attribute.PosixFileAttributeView; import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.EnumSet; import java.util.List; import java.util.Set; import java.util.UUID; @@ -46,6 +50,8 @@ /** Security Configuration Handler which works on standalone local files */ public class SecurityConfHandlerLocal extends SecurityConfHandler { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private static final Set OWNER_READ_WRITE = + EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE); protected Path securityJsonPath; private final Object securityJsonLock = new Object(); @@ -126,8 +132,9 @@ private void replaceSecurityJson(byte[] json) throws IOException { Path tmpPath = securityJsonPath.resolveSibling( securityJsonPath.getFileName() + "." + UUID.randomUUID() + ".tmp"); + boolean replacing = Files.exists(securityJsonPath); try { - Files.createFile(tmpPath); + Files.createFile(tmpPath, temporaryFileAttributes(replacing)); } catch (AccessDeniedException e) { log.warn( "Could not create a temporary file next to {}, writing it in place", securityJsonPath, e); @@ -170,6 +177,12 @@ private void replaceSecurityJson(byte[] json) throws IOException { } } + private static FileAttribute[] temporaryFileAttributes(boolean replacing) { + return replacing + ? new FileAttribute[] {PosixFilePermissions.asFileAttribute(OWNER_READ_WRITE)} + : new FileAttribute[0]; + } + private void writeInPlace(byte[] json) throws IOException { try (OutputStream securityJsonOs = Files.newOutputStream(securityJsonPath)) { securityJsonOs.write(json); From 46869c4d8bc9ab26366a6ba97ba884bc2040911b Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Fri, 23 Oct 2026 01:00:48 +0000 Subject: [PATCH 11/12] SOLR-18010: test the failure paths with a file system that fails on demand --- .../SecurityConfHandlerFailurePathsTest.java | 464 ++++++++++++++++++ 1 file changed, 464 insertions(+) create mode 100644 solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java new file mode 100644 index 000000000000..9e61e49c5e98 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java @@ -0,0 +1,464 @@ +/* + * 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.solr.handler.admin; + +import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.nio.channels.FileChannel; +import java.nio.charset.StandardCharsets; +import java.nio.file.CopyOption; +import java.nio.file.FileSystem; +import java.nio.file.FileSystemException; +import java.nio.file.Files; +import java.nio.file.LinkOption; +import java.nio.file.OpenOption; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.FileAttributeView; +import java.nio.file.attribute.FileOwnerAttributeView; +import java.nio.file.attribute.FileTime; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFileAttributes; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.UserPrincipal; +import java.util.ArrayList; +import java.util.EnumSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import org.apache.lucene.tests.mockfile.FilterFileChannel; +import org.apache.lucene.tests.mockfile.FilterFileSystemProvider; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.ContentStreamBase; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.request.SolrQueryRequestBase; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.security.RuleBasedAuthorizationPlugin; + +/** + * Failure paths of the temporary-file write, forced through a file system that wraps the default + * one. The production code is not changed for the tests. + */ +public class SecurityConfHandlerFailurePathsTest extends SolrTestCase { + + private static final String EDIT = "{'set-user-role': {'alice': 'admin'}}"; + private static final String FAKE_OWNER_NAME = "other-owner"; + private static final String FAKE_GROUP_NAME = "other-group"; + private static final UserPrincipal FAKE_OWNER = () -> FAKE_OWNER_NAME; + private static final GroupPrincipal FAKE_GROUP = () -> FAKE_GROUP_NAME; + + private enum Fault { + SYNC_TEMP_FILE, + SYNC_DIRECTORY, + MOVE, + PERMISSION_COPY, + OWNER_MISMATCH, + GROUP_COPY, + GROUP_SET_FAILS + } + + private FaultyFileSystemProvider faulty; + private Path solrHome; + private Path securityJson; + + public void testFailedSyncKeepsExistingSecurityJson() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsSync"); + byte[] seed = Files.readAllBytes(securityJson); + faulty.faults.add(Fault.SYNC_TEMP_FILE); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + assertThrows(SolrException.class, () -> postAuthorizationEdit(handler, EDIT)); + assertFalse("the file was moved after a failed sync", faulty.events.contains("move")); + assertArrayEquals(seed, Files.readAllBytes(securityJson)); + assertNoTemporaryFiles(solrHome); + } + + public void testTemporaryFileIsSyncedBeforeItIsMoved() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsOrder"); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertEquals(List.of("sync", "move", "dirSync"), faulty.events); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertNoTemporaryFiles(solrHome); + } + + public void testDirectorySyncFailureAfterMoveDoesNotFailEdit() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsDirSync"); + faulty.faults.add(Fault.SYNC_DIRECTORY); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertEquals(List.of("sync", "move", "dirSync"), faulty.events); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertNoTemporaryFiles(solrHome); + } + + public void testRefusedMoveFallsBackToInPlaceWrite() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsMove"); + Object fileKey = fileKey(); + faulty.faults.add(Fault.MOVE); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertTrue("the move was not attempted", faulty.events.contains("move")); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertEquals("security.json was replaced, not written in place", fileKey, fileKey()); + assertNoTemporaryFiles(solrHome); + } + + public void testFailedPermissionCopyKeepsExistingSecurityJson() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsPerms"); + byte[] seed = Files.readAllBytes(securityJson); + faulty.faults.add(Fault.PERMISSION_COPY); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + assertThrows(SolrException.class, () -> postAuthorizationEdit(handler, EDIT)); + assertFalse( + "the file was moved after a failed permission copy", faulty.events.contains("move")); + assertArrayEquals(seed, Files.readAllBytes(securityJson)); + assertNoTemporaryFiles(solrHome); + } + + public void testOwnerMismatchFallsBackToInPlaceWrite() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsOwner"); + Object fileKey = fileKey(); + faulty.faults.add(Fault.OWNER_MISMATCH); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertFalse("the file was replaced for another owner", faulty.events.contains("move")); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertEquals(fileKey, fileKey()); + assertNoTemporaryFiles(solrHome); + } + + public void testGroupIsCopiedToTemporaryFile() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsGroup"); + Object fileKey = fileKey(); + faulty.faults.add(Fault.GROUP_COPY); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertEquals(List.of(FAKE_GROUP_NAME), faulty.groupsSet); + assertTrue(faulty.events.contains("move")); + assertNotEquals("security.json was not replaced", fileKey, fileKey()); + assertNoTemporaryFiles(solrHome); + } + + public void testFailedGroupCopyFallsBackToInPlaceWrite() throws Exception { + assumeWorkingMockito(); + setUpSolrHome("securityConfHandlerFaultsGroupFail"); + Object fileKey = fileKey(); + faulty.faults.add(Fault.GROUP_COPY); + faulty.faults.add(Fault.GROUP_SET_FAILS); + SecurityConfHandlerLocal handler = new SecurityConfHandlerLocal(mockCoreContainer(solrHome)); + postAuthorizationEdit(handler, EDIT); + assertFalse("the file was replaced after a failed group copy", faulty.events.contains("move")); + assertTrue(Files.readString(securityJson, StandardCharsets.UTF_8).contains("alice")); + assertEquals(fileKey, fileKey()); + assertNoTemporaryFiles(solrHome); + } + + private void setUpSolrHome(String name) throws IOException { + Path base = createTempDir(name); + assumeTrue( + "POSIX file permissions are not supported on this file system", + base.getFileSystem().supportedFileAttributeViews().contains("posix")); + faulty = new FaultyFileSystemProvider(base.getFileSystem()); + solrHome = faulty.wrapPath(base); + securityJson = solrHome.resolve("security.json"); + Files.write(securityJson, seedSecurityJson().getBytes(StandardCharsets.UTF_8)); + } + + private Object fileKey() throws IOException { + return Files.readAttributes(securityJson, BasicFileAttributes.class).fileKey(); + } + + private static String seedSecurityJson() { + return "{\"authentication\":{\"class\":\"solr.BasicAuthPlugin\"," + + "\"credentials\":{\"solr\":\"solr\"}}," + + "\"authorization\":{\"class\":\"solr.RuleBasedAuthorizationPlugin\",\"user-role\":{}}}"; + } + + private static CoreContainer mockCoreContainer(Path solrHome) { + CoreContainer cc = mock(CoreContainer.class); + when(cc.getSolrHome()).thenReturn(solrHome); + when(cc.getAuthorizationPlugin()).thenReturn(new RuleBasedAuthorizationPlugin()); + return cc; + } + + private static void assertNoTemporaryFiles(Path solrHome) throws IOException { + try (Stream files = Files.list(solrHome)) { + assertEquals( + "temporary files left behind", + List.of(), + files.filter(f -> f.getFileName().toString().endsWith(".tmp")).toList()); + } + } + + private static void postAuthorizationEdit(SecurityConfHandler handler, String command) + throws Exception { + SolrQueryRequestBase req = new SolrQueryRequestBase(null, new ModifiableSolrParams()); + req.getContext().put("httpMethod", SolrRequest.METHOD.POST); + req.getContext().put("path", "/admin/authorization"); + req.setContentStreams( + List.of( + new ContentStreamBase.ByteArrayStream(command.getBytes(StandardCharsets.UTF_8), ""))); + handler.handleRequestBody(req, new SolrQueryResponse()); + } + + private static boolean isSecurityJson(Path path) { + return path.getFileName().toString().equals("security.json"); + } + + private static boolean isTemporary(Path path) { + return path.getFileName().toString().endsWith(".tmp"); + } + + private static final class FaultyFileSystemProvider extends FilterFileSystemProvider { + final Set faults = EnumSet.noneOf(Fault.class); + final List events = new ArrayList<>(); + final List groupsSet = new ArrayList<>(); + + FaultyFileSystemProvider(FileSystem delegate) { + super("faulty", delegate); + } + + @Override + public FileChannel newFileChannel( + Path path, Set options, FileAttribute... attrs) + throws IOException { + FileChannel channel = super.newFileChannel(path, options, attrs); + boolean temporary = isTemporary(path); + boolean directory = !temporary && Files.isDirectory(path); + return new FilterFileChannel(channel) { + @Override + public void force(boolean metaData) throws IOException { + if (temporary) { + events.add("sync"); + if (faults.contains(Fault.SYNC_TEMP_FILE)) { + throw new IOException("injected sync failure"); + } + } else if (directory) { + events.add("dirSync"); + if (faults.contains(Fault.SYNC_DIRECTORY)) { + throw new IOException("injected directory sync failure"); + } + } + super.force(metaData); + } + }; + } + + @Override + public void move(Path source, Path target, CopyOption... options) throws IOException { + events.add("move"); + if (faults.contains(Fault.MOVE) && isSecurityJson(target)) { + throw new FileSystemException(target.toString(), null, "injected refused move"); + } + super.move(source, target, options); + } + + @Override + public A readAttributes( + Path path, Class type, LinkOption... options) throws IOException { + A attributes = super.readAttributes(path, type, options); + if (type == PosixFileAttributes.class + && faults.contains(Fault.GROUP_COPY) + && isSecurityJson(path)) { + return type.cast(new OtherGroupAttributes((PosixFileAttributes) attributes)); + } + return attributes; + } + + @Override + public V getFileAttributeView( + Path path, Class type, LinkOption... options) { + V view = super.getFileAttributeView(path, type, options); + if (type == PosixFileAttributeView.class) { + return type.cast(new PosixView((PosixFileAttributeView) view, path)); + } + if (type == FileOwnerAttributeView.class) { + return type.cast(new OwnerView((FileOwnerAttributeView) view, path)); + } + return view; + } + + private final class PosixView implements PosixFileAttributeView { + private final PosixFileAttributeView view; + private final Path path; + + PosixView(PosixFileAttributeView view, Path path) { + this.view = view; + this.path = path; + } + + @Override + public String name() { + return view.name(); + } + + @Override + public PosixFileAttributes readAttributes() throws IOException { + return view.readAttributes(); + } + + @Override + public void setTimes(FileTime lastModifiedTime, FileTime lastAccessTime, FileTime createTime) + throws IOException { + view.setTimes(lastModifiedTime, lastAccessTime, createTime); + } + + @Override + public void setPermissions(Set perms) throws IOException { + if (faults.contains(Fault.PERMISSION_COPY) && isTemporary(path)) { + throw new IOException("injected permission copy failure"); + } + view.setPermissions(perms); + } + + @Override + public void setOwner(UserPrincipal owner) throws IOException { + view.setOwner(owner); + } + + @Override + public UserPrincipal getOwner() throws IOException { + return view.getOwner(); + } + + @Override + public void setGroup(GroupPrincipal group) throws IOException { + if (isTemporary(path)) { + groupsSet.add(group.getName()); + if (faults.contains(Fault.GROUP_SET_FAILS)) { + throw new IOException("injected group failure"); + } + } + if (group == FAKE_GROUP) { + return; + } + view.setGroup(group); + } + } + + private final class OwnerView implements FileOwnerAttributeView { + private final FileOwnerAttributeView view; + private final Path path; + + OwnerView(FileOwnerAttributeView view, Path path) { + this.view = view; + this.path = path; + } + + @Override + public String name() { + return view.name(); + } + + @Override + public UserPrincipal getOwner() throws IOException { + if (faults.contains(Fault.OWNER_MISMATCH) && isSecurityJson(path)) { + return FAKE_OWNER; + } + return view.getOwner(); + } + + @Override + public void setOwner(UserPrincipal owner) throws IOException { + view.setOwner(owner); + } + } + } + + private static final class OtherGroupAttributes implements PosixFileAttributes { + private final PosixFileAttributes attributes; + + OtherGroupAttributes(PosixFileAttributes attributes) { + this.attributes = attributes; + } + + @Override + public GroupPrincipal group() { + return FAKE_GROUP; + } + + @Override + public UserPrincipal owner() { + return attributes.owner(); + } + + @Override + public Set permissions() { + return attributes.permissions(); + } + + @Override + public FileTime lastModifiedTime() { + return attributes.lastModifiedTime(); + } + + @Override + public FileTime lastAccessTime() { + return attributes.lastAccessTime(); + } + + @Override + public FileTime creationTime() { + return attributes.creationTime(); + } + + @Override + public boolean isRegularFile() { + return attributes.isRegularFile(); + } + + @Override + public boolean isDirectory() { + return attributes.isDirectory(); + } + + @Override + public boolean isSymbolicLink() { + return attributes.isSymbolicLink(); + } + + @Override + public boolean isOther() { + return attributes.isOther(); + } + + @Override + public long size() { + return attributes.size(); + } + + @Override + public Object fileKey() { + return attributes.fileKey(); + } + } +} From d742eb99dd28be761b5f60aea6fbfc832e1b7a50 Mon Sep 17 00:00:00 2001 From: Nick Shanin Date: Fri, 23 Oct 2026 01:55:57 +0000 Subject: [PATCH 12/12] SOLR-18010: compare the test group with Objects.equals for Error Prone --- .../handler/admin/SecurityConfHandlerFailurePathsTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java index 9e61e49c5e98..c64bee61d0f7 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/SecurityConfHandlerFailurePathsTest.java @@ -43,6 +43,7 @@ import java.util.ArrayList; import java.util.EnumSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.stream.Stream; import org.apache.lucene.tests.mockfile.FilterFileChannel; @@ -358,7 +359,7 @@ public void setGroup(GroupPrincipal group) throws IOException { throw new IOException("injected group failure"); } } - if (group == FAKE_GROUP) { + if (Objects.equals(group, FAKE_GROUP)) { return; } view.setGroup(group);