Skip to content

Commit faf0b7d

Browse files
sfc-gh-mkubiksfc-gh-mkubik
authored
SNOW-1660409 revert of revert of strict permissions on get (#2232)
1 parent 7cbaabf commit faf0b7d

10 files changed

Lines changed: 202 additions & 4 deletions

File tree

‎src/main/java/net/snowflake/client/core/SFBaseSession.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ public abstract class SFBaseSession {
154154
/** Treat java.sql.Time as wall clock time without converting it to UTC */
155155
private boolean treatTimeAsWallClockTime = false;
156156

157+
private boolean ownerOnlyStageFilePermissionsEnabled = false;
158+
157159
protected SFBaseSession(SFConnectionHandler sfConnectionHandler) {
158160
this.sfConnectionHandler = sfConnectionHandler;
159161
}
@@ -1374,4 +1376,17 @@ public boolean getTreatTimeAsWallClockTime() {
13741376
public void setTreatTimeAsWallClockTime(boolean treatTimeAsWallClockTime) {
13751377
this.treatTimeAsWallClockTime = treatTimeAsWallClockTime;
13761378
}
1379+
1380+
/**
1381+
* Get if owner-only stage file permissions feature is enabled.
1382+
*
1383+
* @return true if owner-only stage file permissions feature is enabled
1384+
*/
1385+
public boolean isOwnerOnlyStageFilePermissionsEnabled() {
1386+
return ownerOnlyStageFilePermissionsEnabled;
1387+
}
1388+
1389+
public void setOwnerOnlyStageFilePermissionsEnabled(boolean booleanValue) {
1390+
this.ownerOnlyStageFilePermissionsEnabled = booleanValue;
1391+
}
13771392
}

‎src/main/java/net/snowflake/client/core/SFSession.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,12 @@ public void addSFSessionProperty(String propertyName, Object propertyValue) thro
578578
}
579579
break;
580580

581+
case OWNER_ONLY_STAGE_FILE_PERMISSIONS_ENABLED:
582+
if (propertyValue != null) {
583+
setOwnerOnlyStageFilePermissionsEnabled(getBooleanValue(propertyValue));
584+
}
585+
break;
586+
581587
default:
582588
break;
583589
}

‎src/main/java/net/snowflake/client/core/SFSessionProperty.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,12 @@ public enum SFSessionProperty {
137137
"CLIENT_TREAT_TIME_AS_WALL_CLOCK_TIME", false, Boolean.class),
138138

139139
HTTP_HEADER_CUSTOMIZERS(
140-
HttpHeadersCustomizer.HTTP_HEADER_CUSTOMIZERS_PROPERTY_KEY, false, List.class);
140+
HttpHeadersCustomizer.HTTP_HEADER_CUSTOMIZERS_PROPERTY_KEY, false, List.class),
141+
142+
// Used to enable the owner-only stage file permissions feature. This feature will be enabled by
143+
// default in the next major release.
144+
OWNER_ONLY_STAGE_FILE_PERMISSIONS_ENABLED(
145+
"ownerOnlyStageFilePermissionsEnabled", false, Boolean.class);
141146

142147
// property key in string
143148
private String propertyKey;

‎src/main/java/net/snowflake/client/jdbc/SnowflakeFileTransferAgent.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package net.snowflake.client.jdbc;
22

33
import static net.snowflake.client.core.Constants.NO_SPACE_LEFT_ON_DEVICE_ERR;
4+
import static net.snowflake.client.jdbc.SnowflakeUtil.createOwnerOnlyPermissionDir;
45
import static net.snowflake.client.jdbc.SnowflakeUtil.isNullOrEmpty;
56
import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty;
67

@@ -1572,7 +1573,12 @@ public boolean execute() throws SQLException {
15721573
if (commandType == CommandType.DOWNLOAD) {
15731574
File dir = new File(localLocation);
15741575
if (!dir.exists()) {
1575-
boolean created = dir.mkdirs();
1576+
boolean created;
1577+
if (session.isOwnerOnlyStageFilePermissionsEnabled()) {
1578+
created = createOwnerOnlyPermissionDir(localLocation);
1579+
} else {
1580+
created = dir.mkdirs();
1581+
}
15761582
if (created) {
15771583
logger.debug("Directory created: {}", localLocation);
15781584
} else {
@@ -2517,7 +2523,7 @@ private static void pullFileFromRemoteStore(
25172523
RemoteStoreFileEncryptionMaterial encMat,
25182524
String presignedUrl,
25192525
String queryId)
2520-
throws SQLException {
2526+
throws SQLException, IOException {
25212527
remoteLocation remoteLocation = extractLocationAndPath(stage.getLocation());
25222528

25232529
String stageFilePath = filePath;

‎src/main/java/net/snowflake/client/jdbc/SnowflakeUtil.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,18 @@
99
import com.fasterxml.jackson.databind.ObjectMapper;
1010
import com.fasterxml.jackson.databind.node.ArrayNode;
1111
import java.io.BufferedReader;
12+
import java.io.File;
1213
import java.io.IOException;
1314
import java.io.InputStreamReader;
1415
import java.io.PrintWriter;
1516
import java.io.StringWriter;
1617
import java.lang.reflect.Field;
1718
import java.lang.reflect.Method;
19+
import java.nio.file.Files;
20+
import java.nio.file.Path;
21+
import java.nio.file.Paths;
22+
import java.nio.file.attribute.PosixFilePermission;
23+
import java.nio.file.attribute.PosixFilePermissions;
1824
import java.sql.SQLException;
1925
import java.sql.Time;
2026
import java.sql.Types;
@@ -29,6 +35,7 @@
2935
import java.util.Optional;
3036
import java.util.Properties;
3137
import java.util.Random;
38+
import java.util.Set;
3239
import java.util.TreeMap;
3340
import java.util.concurrent.Executors;
3441
import java.util.concurrent.ThreadFactory;
@@ -59,6 +66,9 @@ public class SnowflakeUtil {
5966
private static final SFLogger logger = SFLoggerFactory.getLogger(SnowflakeUtil.class);
6067
private static final ObjectMapper OBJECT_MAPPER = ObjectMapperFactory.getObjectMapper();
6168

69+
private static final Set<PosixFilePermission> directoryOwnerOnlyPermission =
70+
PosixFilePermissions.fromString("rwx------");
71+
6272
/** Additional data types not covered by standard JDBC */
6373
public static final int EXTRA_TYPES_TIMESTAMP_LTZ = 50000;
6474

@@ -894,6 +904,58 @@ public static Map<String, String> createCaseInsensitiveMap(Header[] headers) {
894904
}
895905
}
896906

907+
/** create a directory with Owner only permission (0600) */
908+
@SnowflakeJdbcInternalApi
909+
public static boolean createOwnerOnlyPermissionDir(String location) {
910+
if (isWindows()) {
911+
File dir = new File(location);
912+
return dir.mkdirs();
913+
}
914+
915+
boolean isDirCreated = true;
916+
Path dir = Paths.get(location);
917+
try {
918+
Files.createDirectory(
919+
dir, PosixFilePermissions.asFileAttribute(directoryOwnerOnlyPermission));
920+
} catch (IOException e) {
921+
logger.error(
922+
"Failed to set OwnerOnly permission for {}. This may cause the file download to fail ",
923+
location);
924+
isDirCreated = false;
925+
}
926+
return isDirCreated;
927+
}
928+
929+
@SnowflakeJdbcInternalApi
930+
public static void assureOnlyUserAccessibleFilePermissions(
931+
File file, boolean isOwnerOnlyStageFilePermissionsEnabled) throws IOException {
932+
if (isWindows()) {
933+
return;
934+
}
935+
if (!isOwnerOnlyStageFilePermissionsEnabled) {
936+
// If the owner only stage file permissions are not enabled, we do not need to set the file
937+
// permissions.
938+
return;
939+
}
940+
boolean disableUserPermissions =
941+
file.setReadable(false, false)
942+
&& file.setWritable(false, false)
943+
&& file.setExecutable(false, false);
944+
boolean setOwnerPermissionsOnly = file.setReadable(true, true) && file.setWritable(true, true);
945+
946+
if (disableUserPermissions && setOwnerPermissionsOnly) {
947+
logger.info("Successfuly set OwnerOnly permission for {}. ", file.getAbsolutePath());
948+
} else {
949+
file.delete();
950+
logger.error(
951+
"Failed to set OwnerOnly permission for {}. Failed to download", file.getAbsolutePath());
952+
throw new IOException(
953+
String.format(
954+
"Failed to set OwnerOnly permission for %s. Failed to download",
955+
file.getAbsolutePath()));
956+
}
957+
}
958+
897959
/**
898960
* Check whether the OS is Windows
899961
*

‎src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeAzureClient.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,9 @@ public void download(
336336
transferOptions.setConcurrentRequestCount(parallelism);
337337

338338
blob.downloadToFile(localFilePath, null, transferOptions, opContext);
339+
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(
340+
localFile, session.isOwnerOnlyStageFilePermissionsEnabled());
341+
339342
stopwatch.stop();
340343
long downloadMillis = stopwatch.elapsedMillis();
341344

‎src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeGCSClient.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import net.snowflake.client.jdbc.SnowflakeFileTransferAgent;
4343
import net.snowflake.client.jdbc.SnowflakeSQLException;
4444
import net.snowflake.client.jdbc.SnowflakeSQLLoggedException;
45+
import net.snowflake.client.jdbc.SnowflakeUtil;
4546
import net.snowflake.client.log.ArgSupplier;
4647
import net.snowflake.client.log.SFLogger;
4748
import net.snowflake.client.log.SFLoggerFactory;
@@ -270,6 +271,8 @@ public void download(
270271
outStream.flush();
271272
outStream.close();
272273
bodyStream.close();
274+
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(
275+
localFile, session.isOwnerOnlyStageFilePermissionsEnabled());
273276
if (isEncrypting()) {
274277
Map<String, String> userDefinedHeaders =
275278
createCaseInsensitiveMap(response.getAllHeaders());
@@ -298,7 +301,8 @@ public void download(
298301
Map<String, String> userDefinedMetadata =
299302
this.gcsAccessStrategy.download(
300303
parallelism, remoteStorageLocation, stageFilePath, localFile);
301-
304+
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(
305+
localFile, session.isOwnerOnlyStageFilePermissionsEnabled());
302306
stopwatch.stop();
303307
downloadMillis = stopwatch.elapsedMillis();
304308
logger.debug("Download successful", false);

‎src/main/java/net/snowflake/client/jdbc/cloud/storage/SnowflakeS3Client.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,8 @@ public ExecutorService newExecutor() {
391391
String iv = metaMap.get(AMZ_IV);
392392

393393
myDownload.waitForCompletion();
394+
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(
395+
localFile, session.isOwnerOnlyStageFilePermissionsEnabled());
394396
stopwatch.stop();
395397
long downloadMillis = stopwatch.elapsedMillis();
396398

‎src/test/java/net/snowflake/client/jdbc/FileUploaderLatestIT.java‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
import java.nio.file.Files;
2222
import java.nio.file.Path;
2323
import java.nio.file.Paths;
24+
import java.nio.file.attribute.PosixFileAttributes;
25+
import java.nio.file.attribute.PosixFilePermission;
26+
import java.nio.file.attribute.PosixFilePermissions;
2427
import java.security.NoSuchAlgorithmException;
2528
import java.sql.Connection;
2629
import java.sql.ResultSet;
@@ -29,7 +32,9 @@
2932
import java.util.List;
3033
import java.util.Map;
3134
import java.util.Properties;
35+
import java.util.Set;
3236
import net.snowflake.client.annotations.DontRunOnGithubActions;
37+
import net.snowflake.client.annotations.DontRunOnWindows;
3338
import net.snowflake.client.category.TestTags;
3439
import net.snowflake.client.core.OCSPMode;
3540
import net.snowflake.client.core.SFSession;
@@ -46,6 +51,8 @@
4651
import org.junit.jupiter.api.Tag;
4752
import org.junit.jupiter.api.Test;
4853
import org.junit.jupiter.api.io.TempDir;
54+
import org.junit.jupiter.params.ParameterizedTest;
55+
import org.junit.jupiter.params.provider.CsvSource;
4956

5057
/** Tests for SnowflakeFileTransferAgent that require an active connection */
5158
@Tag(TestTags.OTHERS)
@@ -906,4 +913,54 @@ public void testUploadWithTripleSlashFilePrefix(@TempDir File tempDir)
906913
}
907914
}
908915
}
916+
917+
@DontRunOnWindows
918+
@ParameterizedTest
919+
@CsvSource({"true, rwx------, rw-------", "false, rwxr-xr-x, rw-r--r--"})
920+
public void testDownloadedFilePermissions(
921+
boolean ownerOnlyStageFilePermissionsEnabled,
922+
String expectedDirectoryPermissions,
923+
String expectedFilePermissions,
924+
@TempDir File tempDir)
925+
throws SQLException, IOException {
926+
String stageName = "testStage" + SnowflakeUtil.randomAlphaNumeric(10);
927+
try (Connection connection = getConnection();
928+
Statement statement = connection.createStatement()) {
929+
try {
930+
statement.execute("CREATE OR REPLACE STAGE " + stageName);
931+
SFSession sfSession = connection.unwrap(SnowflakeConnectionV1.class).getSfSession();
932+
sfSession.setOwnerOnlyStageFilePermissionsEnabled(ownerOnlyStageFilePermissionsEnabled);
933+
934+
String command =
935+
"PUT file:///"
936+
+ getFullPathFileInResource(TEST_DATA_FILE)
937+
+ " @"
938+
+ stageName
939+
+ " AUTO_COMPRESS=FALSE";
940+
SnowflakeFileTransferAgent sfAgent =
941+
new SnowflakeFileTransferAgent(command, sfSession, new SFStatement(sfSession));
942+
assertTrue(sfAgent.execute());
943+
944+
String tempDirPath = (tempDir.getCanonicalPath() + "/new_directory").replace("\\", "/");
945+
String getCommand = "GET @" + stageName + " file://" + tempDirPath;
946+
SnowflakeFileTransferAgent sfAgent1 =
947+
new SnowflakeFileTransferAgent(getCommand, sfSession, new SFStatement(sfSession));
948+
assertTrue(sfAgent1.execute());
949+
assertEquals(1, sfAgent1.statusRows.size());
950+
File downloadedFile = new File(tempDirPath, TEST_DATA_FILE);
951+
PosixFileAttributes dirAttributes =
952+
Files.readAttributes(new File(tempDirPath).toPath(), PosixFileAttributes.class);
953+
PosixFileAttributes fileAttributes =
954+
Files.readAttributes(downloadedFile.toPath(), PosixFileAttributes.class);
955+
Set<PosixFilePermission> tmpDirPermissions = dirAttributes.permissions();
956+
Set<PosixFilePermission> filePermissions = fileAttributes.permissions();
957+
958+
assertEquals(
959+
PosixFilePermissions.fromString(expectedDirectoryPermissions), tmpDirPermissions);
960+
assertEquals(PosixFilePermissions.fromString(expectedFilePermissions), filePermissions);
961+
} finally {
962+
statement.execute("DROP STAGE if exists " + stageName);
963+
}
964+
}
965+
}
909966
}

‎src/test/java/net/snowflake/client/jdbc/SnowflakeUtilTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package net.snowflake.client.jdbc;
22

33
import static net.snowflake.client.jdbc.SnowflakeUtil.createCaseInsensitiveMap;
4+
import static net.snowflake.client.jdbc.SnowflakeUtil.createOwnerOnlyPermissionDir;
45
import static net.snowflake.client.jdbc.SnowflakeUtil.extractColumnMetadata;
56
import static net.snowflake.client.jdbc.SnowflakeUtil.getSnowflakeType;
67
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -12,18 +13,28 @@
1213
import com.fasterxml.jackson.databind.ObjectMapper;
1314
import com.fasterxml.jackson.databind.node.ArrayNode;
1415
import com.fasterxml.jackson.databind.node.ObjectNode;
16+
import java.io.File;
17+
import java.io.IOException;
18+
import java.nio.file.Files;
19+
import java.nio.file.Path;
20+
import java.nio.file.attribute.PosixFileAttributes;
21+
import java.nio.file.attribute.PosixFilePermission;
22+
import java.nio.file.attribute.PosixFilePermissions;
1523
import java.sql.Types;
1624
import java.util.ArrayList;
1725
import java.util.Arrays;
1826
import java.util.HashMap;
1927
import java.util.Map;
28+
import java.util.Set;
2029
import java.util.TreeMap;
30+
import net.snowflake.client.annotations.DontRunOnWindows;
2131
import net.snowflake.client.category.TestTags;
2232
import net.snowflake.client.core.ObjectMapperFactory;
2333
import org.apache.http.Header;
2434
import org.apache.http.message.BasicHeader;
2535
import org.junit.jupiter.api.Tag;
2636
import org.junit.jupiter.api.Test;
37+
import org.junit.jupiter.api.io.TempDir;
2738

2839
@Tag(TestTags.CORE)
2940
public class SnowflakeUtilTest extends BaseJDBCTest {
@@ -114,6 +125,33 @@ public void shouldConvertHeadersCreateCaseInsensitiveMap() {
114125
assertEquals("value2", map.get("Key2"));
115126
}
116127

128+
@Test
129+
@DontRunOnWindows
130+
public void testCreateOwnerOnlyPermissionDir(@TempDir Path tempDir)
131+
throws IOException, UnsupportedOperationException {
132+
Path tmp = tempDir.resolve("folder-permission-testing");
133+
String folderPath = tmp.toString();
134+
boolean isDirCreated = createOwnerOnlyPermissionDir(folderPath);
135+
assertTrue(isDirCreated);
136+
assertTrue(tmp.toFile().isDirectory());
137+
PosixFileAttributes attributes = Files.readAttributes(tmp, PosixFileAttributes.class);
138+
Set<PosixFilePermission> permissions = attributes.permissions();
139+
assertEquals(PosixFilePermissions.toString(permissions), "rwx------");
140+
}
141+
142+
@Test
143+
@DontRunOnWindows
144+
public void testValidateFilePermission(@TempDir Path tempDir)
145+
throws IOException, UnsupportedOperationException {
146+
Path tmp = tempDir.resolve("fileTesting.txt");
147+
File file = tmp.toFile();
148+
assertTrue(file.createNewFile());
149+
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(file, true);
150+
PosixFileAttributes attributes = Files.readAttributes(tmp, PosixFileAttributes.class);
151+
Set<PosixFilePermission> permissions = attributes.permissions();
152+
assertEquals(PosixFilePermissions.toString(permissions), "rw-------");
153+
}
154+
117155
private static SnowflakeColumnMetadata createExpectedMetadata(
118156
JsonNode rootNode, JsonNode fieldOne, JsonNode fieldTwo) throws SnowflakeSQLLoggedException {
119157
ColumnTypeInfo columnTypeInfo =

0 commit comments

Comments
 (0)