Merge branch 'main' of github.com:line/centraldogma into readonly-per-repository

Author: ikhoon

it/mirror/src/test/java/com/linecorp/centraldogma/it/mirror/git/CentralDogmaMirrorTest.java

@@ -27,6 +27,7 @@
 import java.util.Map;
 import java.util.concurrent.CompletionException;
 
+import org.jspecify.annotations.Nullable;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -281,11 +282,66 @@ void localToRemote_tooManyFiles() throws Exception {
                 .hasMessageContaining("file");
     }
 
+    @Test
+    void localToRemote_gitignore() throws Exception {
+        pushMirrorSettings("/", "/", MirrorDirection.LOCAL_TO_REMOTE, "*.log\n/build/");
+
+        // Add files - some match gitignore patterns.
+        localClient.forRepo(projName, REPO_FOO)
+                   .commit("Add files",
+                           Change.ofTextUpsert("/app.txt", "app content"),
+                           Change.ofTextUpsert("/error.log", "log content"),
+                           Change.ofTextUpsert("/build/output.txt", "build output"),
+                           Change.ofJsonUpsert("/config.json", "{\"key\":\"value\"}"))
+                   .push().join();
+
+        mirroringService.mirror().join();
+
+        final Map<String, Entry<?>> remoteEntries =
+                remoteClient.getFiles(projName, REPO_FOO, Revision.HEAD, PathPattern.all()).join();
+        // Non-ignored files should be mirrored.
+        assertThat(remoteEntries).containsKey("/app.txt");
+        assertThat(remoteEntries).containsKey("/config.json");
+        // Ignored files should NOT be mirrored.
+        assertThat(remoteEntries).doesNotContainKey("/error.log");
+        assertThat(remoteEntries).doesNotContainKey("/build/output.txt");
+    }
+
+    @Test
+    void remoteToLocal_gitignore() throws Exception {
+        pushMirrorSettings("/", "/", MirrorDirection.REMOTE_TO_LOCAL, "*.log\n/build/");
+
+        // Add files to remote - some match gitignore patterns.
+        remoteClient.forRepo(projName, REPO_FOO)
+                    .commit("Add files",
+                            Change.ofTextUpsert("/app.txt", "app content"),
+                            Change.ofTextUpsert("/error.log", "log content"),
+                            Change.ofTextUpsert("/build/output.txt", "build output"),
+                            Change.ofJsonUpsert("/config.json", "{\"key\":\"value\"}"))
+                    .push().join();
+
+        mirroringService.mirror().join();
+
+        final Map<String, Entry<?>> localEntries =
+                localClient.getFiles(projName, REPO_FOO, Revision.HEAD, PathPattern.all()).join();
+        // Non-ignored files should be mirrored.
+        assertThat(localEntries).containsKey("/app.txt");
+        assertThat(localEntries).containsKey("/config.json");
+        // Ignored files should NOT be mirrored.
+        assertThat(localEntries).doesNotContainKey("/error.log");
+        assertThat(localEntries).doesNotContainKey("/build/output.txt");
+    }
+
     private void pushMirrorSettings(String localPath, String remotePath) {
-        pushMirrorSettings(localPath, remotePath, MirrorDirection.LOCAL_TO_REMOTE);
+        pushMirrorSettings(localPath, remotePath, MirrorDirection.LOCAL_TO_REMOTE, null);
     }
 
     private void pushMirrorSettings(String localPath, String remotePath, MirrorDirection direction) {
+        pushMirrorSettings(localPath, remotePath, direction, null);
+    }
+
+    private void pushMirrorSettings(String localPath, String remotePath,
+                                    MirrorDirection direction, @Nullable String gitignore) {
         final InetSocketAddress remoteAddr = remoteDogma.serverAddress();
         final String remoteUri = "dogma://" + remoteAddr.getHostString() + ':' + remoteAddr.getPort() +
                                  '/' + projName + '/' + REPO_FOO + ".dogma" + remotePath;
@@ -306,20 +362,26 @@ private void pushMirrorSettings(String localPath, String remotePath, MirrorDirec
             }
         }
 
+        final StringBuilder config = new StringBuilder();
+        config.append('{')
+              .append("  \"id\": \"foo\",")
+              .append("  \"enabled\": true,")
+              .append("  \"direction\": \"").append(direction).append("\",")
+              .append("  \"localRepo\": \"").append(REPO_FOO).append("\",")
+              .append("  \"localPath\": \"").append(localPath).append("\",")
+              .append("  \"remoteUri\": \"").append(remoteUri).append("\",")
+              .append("  \"schedule\": \"0 0 0 1 1 ? 2099\",")
+              .append("  \"credentialName\": \"").append(credName).append('"');
+        if (gitignore != null) {
+            config.append(",  \"gitignore\": \"").append(gitignore.replace("\n", "\\n")).append('"');
+        }
+        config.append('}');
+
         localClient.forRepo(projName, Project.REPO_DOGMA)
                    .commit("Add mirror config",
                            Change.ofJsonUpsert(
                                    "/repos/" + REPO_FOO + "/mirrors/foo.json",
-                                   '{' +
-                                   "  \"id\": \"foo\"," +
-                                   "  \"enabled\": true," +
-                                   "  \"direction\": \"" + direction + "\"," +
-                                   "  \"localRepo\": \"" + REPO_FOO + "\"," +
-                                   "  \"localPath\": \"" + localPath + "\"," +
-                                   "  \"remoteUri\": \"" + remoteUri + "\"," +
-                                   "  \"schedule\": \"0 0 0 1 1 ? 2099\"," +
-                                   "  \"credentialName\": \"" + credName + '"' +
-                                   '}'))
+                                   config.toString()))
                    .push().join();
     }
 }

server-auth/saml/src/main/java/com/linecorp/centraldogma/server/auth/saml/SamlAuthSsoHandler.java

@@ -40,6 +40,7 @@
 import org.opensaml.saml.saml2.core.Response;
 import org.owasp.encoder.Encode;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 
 import com.linecorp.armeria.common.AggregatedHttpRequest;
@@ -145,10 +146,10 @@ public HttpResponse loginSucceeded(ServiceRequestContext ctx, AggregatedHttpRequ
         final String redirectionScript;
         if (!Strings.isNullOrEmpty(relayState)) {
             final String trimmed = relayState.trim();
-            if (!trimmed.startsWith("/") || trimmed.startsWith("//")) {
-                redirectionScript = "window.location.href='/'";
-            } else {
+            if (isSafeRelayState(trimmed)) {
                 redirectionScript = "window.location.href='" + Encode.forJavaScript(trimmed) + '\'';
+            } else {
+                redirectionScript = "window.location.href='/'";
             }
         } else {
             redirectionScript = "window.location.href='/'";
@@ -173,6 +174,30 @@ public HttpResponse loginSucceeded(ServiceRequestContext ctx, AggregatedHttpRequ
         }));
     }
 
+    /**
+     * Returns whether the specified {@code relayState} is safe to be used as a redirect target. Only a
+     * relative path that starts with exactly one {@code '/'} is allowed in order to prevent an open
+     * redirect. Note that validating the raw string is not enough: a browser normalizes a backslash to
+     * {@code '/'} and strips control characters such as TAB, CR and LF from a URL before navigating, so a
+     * value like {@code "/\evil.example"} or {@code "/\t/evil.example"} would otherwise be resolved to a
+     * protocol-relative URL pointing at an attacker host. Therefore backslashes and control characters are
+     * rejected as well.
+     */
+    @VisibleForTesting
+    static boolean isSafeRelayState(String relayState) {
+        if (!relayState.startsWith("/") || relayState.startsWith("//")) {
+            // Not a relative path, or a protocol-relative URL such as '//evil.example/'.
+            return false;
+        }
+        for (int i = 0; i < relayState.length(); i++) {
+            final char c = relayState.charAt(i);
+            if (c == '\\' || Character.isISOControl(c)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     private HttpResponse httpResponse(LoginResult loginResult, String redirectionScript) {
         final Cookie cookie = createSessionCookie(sessionCookieName, loginResult.sessionCookieValue(),
                                                   tlsEnabled, cookieMaxAgeSecond);

server-auth/saml/src/test/java/com/linecorp/centraldogma/server/auth/saml/SamlAuthSsoHandlerTest.java

@@ -92,6 +92,39 @@ void relayStateIsHtmlEscaped(boolean tlsEnabled) {
         assertCookie(tlsEnabled, aggregated.headers(), "3");
     }
 
+    @ValueSource(strings = {
+            // Absolute URLs and dangerous schemes.
+            "https://evil.example/phish",
+            "javascript:alert(document.domain)",
+            // Protocol-relative URL.
+            "//evil.example/",
+            // Backslash is normalized to '/' by browsers, so this resolves to '//evil.example'.
+            "/\\evil.example",
+            "/\\/evil.example",
+            // Control characters (TAB, CR, LF) are stripped by browsers, so this resolves to
+            // '//evil.example'.
+            "/\t/evil.example",
+            "/\r/evil.example",
+            "/\n/evil.example",
+            // Not a path at all.
+            "evil.example",
+    })
+    @ParameterizedTest
+    void unsafeRelayStateIsRejected(String relayState) {
+        assertThat(SamlAuthSsoHandler.isSafeRelayState(relayState)).isFalse();
+    }
+
+    @ValueSource(strings = {
+            "/",
+            "/dashboard",
+            "/dashboard?next=/home",
+            "/a/b/c",
+    })
+    @ParameterizedTest
+    void relativePathRelayStateIsAccepted(String relayState) {
+        assertThat(SamlAuthSsoHandler.isSafeRelayState(relayState)).isTrue();
+    }
+
     private static void assertCookie(boolean tlsEnabled, ResponseHeaders responseHeaders, String value) {
         final String setCookieValue = responseHeaders.get(HttpHeaderNames.SET_COOKIE);
         final Cookie setCookie = Cookie.fromSetCookieHeader(setCookieValue);

server-auth/shiro/src/main/java/com/linecorp/centraldogma/server/auth/shiro/realm/SearchFirstActiveDirectoryRealm.java

@@ -20,6 +20,7 @@
 import static java.util.Objects.requireNonNull;
 
 import java.time.Duration;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import javax.naming.AuthenticationException;
@@ -157,10 +158,13 @@ protected String findUserDn(LdapContextFactory ldapContextFactory, String userna
             ctrl.setSearchScope(SearchControls.SUBTREE_SCOPE);
             ctrl.setTimeLimit(searchTimeoutMillis);
 
+            // Escape RFC 4515 filter metacharacters to prevent LDAP filter injection.
+            final String escapedUsername = escapeLdapFilterValue(username);
             final String filter =
                     searchFilter != null ? USERNAME_PLACEHOLDER.matcher(searchFilter)
-                                                               .replaceAll(username)
-                                         : username;
+                                                               .replaceAll(Matcher.quoteReplacement(
+                                                                       escapedUsername))
+                                         : escapedUsername;
             final NamingEnumeration<SearchResult> result = ctx.search(searchBase, filter, ctrl);
             try {
                 if (!result.hasMore()) {
@@ -175,6 +179,38 @@ protected String findUserDn(LdapContextFactory ldapContextFactory, String userna
         }
     }
 
+    /**
+     * Escapes the characters that have a special meaning in an LDAP search filter, as defined in
+     * <a href="https://datatracker.ietf.org/doc/html/rfc4515#section-3">RFC 4515 section 3</a>.
+     */
+    static String escapeLdapFilterValue(String value) {
+        requireNonNull(value, "value");
+        final StringBuilder sb = new StringBuilder(value.length());
+        for (int i = 0; i < value.length(); i++) {
+            final char c = value.charAt(i);
+            switch (c) {
+                case '\\':
+                    sb.append("\\5c");
+                    break;
+                case '*':
+                    sb.append("\\2a");
+                    break;
+                case '(':
+                    sb.append("\\28");
+                    break;
+                case ')':
+                    sb.append("\\29");
+                    break;
+                case '\0':
+                    sb.append("\\00");
+                    break;
+                default:
+                    sb.append(c);
+            }
+        }
+        return sb.toString();
+    }
+
     private static UsernamePasswordToken ensureUsernamePasswordToken(AuthenticationToken token) {
         if (token instanceof UsernamePasswordToken) {
             return (UsernamePasswordToken) token;

server-auth/shiro/src/test/java/com/linecorp/centraldogma/server/auth/shiro/realm/SearchFirstActiveDirectoryRealmTest.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright 2026 LINE Corporation
+ *
+ * LINE Corporation 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:
+ *
+ *   https://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 com.linecorp.centraldogma.server.auth.shiro.realm;
+
+import static com.linecorp.centraldogma.server.auth.shiro.realm.SearchFirstActiveDirectoryRealm.escapeLdapFilterValue;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.Test;
+
+class SearchFirstActiveDirectoryRealmTest {
+
+    @Test
+    void escapeLeavesOrdinaryValuesUntouched() {
+        assertThat(escapeLdapFilterValue("alice")).isEqualTo("alice");
+        assertThat(escapeLdapFilterValue("")).isEmpty();
+    }
+
+    @Test
+    void escapeNeutralizesFilterMetacharacters() {
+        assertThat(escapeLdapFilterValue("*")).isEqualTo("\\2a");
+        assertThat(escapeLdapFilterValue("alice)(uid=*"))
+                .isEqualTo("alice\\29\\28uid=\\2a");
+        assertThat(escapeLdapFilterValue("a\\b")).isEqualTo("a\\5cb");
+        assertThat(escapeLdapFilterValue("a\0b")).isEqualTo("a\\00b");
+    }
+}

server-mirror-dogma/src/main/java/com/linecorp/centraldogma/server/internal/mirror/CentralDogmaMirror.java

@@ -140,9 +140,10 @@ protected MirrorResult mirrorLocalToRemote(File workDir, int maxNumFiles, long m
                 return newMirrorResult(MirrorStatus.UP_TO_DATE, description, triggeredTime);
             }
 
-            // Read local files under localPath.
-            final Map<String, Entry<?>> localEntries =
-                    localRepo().find(localHead, localPath() + "**", FIND_ALL_WITH_CONTENT).join();
+            // Read local files under localPath and apply gitignore filter.
+            final Map<String, Entry<?>> localEntries = filterByGitignore(
+                    localRepo().find(localHead, localPath() + "**", FIND_ALL_WITH_CONTENT).join(),
+                    localPath());
 
             // Fetch remote files for comparison/removal detection.
             final Map<String, Entry<?>> remoteEntries =
@@ -279,9 +280,10 @@ protected MirrorResult mirrorRemoteToLocal(File workDir, CommandExecutor executo
                 return newMirrorResult(MirrorStatus.UP_TO_DATE, description, triggeredTime);
             }
 
-            // Fetch all remote files under remotePath.
-            final Map<String, Entry<?>> remoteEntries =
-                    repo.file(PathPattern.of(remotePath() + "**")).viewRaw(true).get(remoteHead).join();
+            // Fetch all remote files under remotePath and apply gitignore filter.
+            final Map<String, Entry<?>> remoteEntries = filterByGitignore(
+                    repo.file(PathPattern.of(remotePath() + "**")).viewRaw(true).get(remoteHead).join(),
+                    remotePath());
 
             // Build Change objects.
             final Map<String, Change<?>> changes = new HashMap<>();