Skip to content

Commit

Permalink
SNOW-1949064 Revert owner-only permissions enforcement for files down…
Browse files Browse the repository at this point in the history
…loaded with GET (#2093)
  • Loading branch information
sfc-gh-mkubik authored Feb 26, 2025
1 parent 5807e94 commit 88bf88a
Show file tree
Hide file tree
Showing 6 changed files with 2 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
package net.snowflake.client.jdbc;

import static net.snowflake.client.core.Constants.NO_SPACE_LEFT_ON_DEVICE_ERR;
import static net.snowflake.client.jdbc.SnowflakeUtil.createOwnerOnlyPermissionDir;
import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty;

import com.fasterxml.jackson.core.JsonProcessingException;
Expand Down Expand Up @@ -1566,10 +1565,7 @@ public boolean execute() throws SQLException {
if (commandType == CommandType.DOWNLOAD) {
File dir = new File(localLocation);
if (!dir.exists()) {
boolean created =
SnowflakeUtil.isWindows()
? dir.mkdirs()
: createOwnerOnlyPermissionDir(localLocation);
boolean created = dir.mkdirs();
if (created) {
logger.debug("Directory created: {}", localLocation);
} else {
Expand Down Expand Up @@ -2510,7 +2506,7 @@ private static void pullFileFromRemoteStore(
RemoteStoreFileEncryptionMaterial encMat,
String presignedUrl,
String queryId)
throws SQLException, IOException {
throws SQLException {
remoteLocation remoteLocation = extractLocationAndPath(stage.getLocation());

String stageFilePath = filePath;
Expand Down
55 changes: 0 additions & 55 deletions src/main/java/net/snowflake/client/jdbc/SnowflakeUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,12 @@
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.google.common.base.Strings;
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.sql.SQLException;
import java.sql.Time;
import java.sql.Types;
Expand All @@ -40,7 +34,6 @@
import java.util.Optional;
import java.util.Properties;
import java.util.Random;
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
Expand Down Expand Up @@ -74,9 +67,6 @@ public class SnowflakeUtil {
private static final SFLogger logger = SFLoggerFactory.getLogger(SnowflakeUtil.class);
private static final ObjectMapper OBJECT_MAPPER = ObjectMapperFactory.getObjectMapper();

private static final Set<PosixFilePermission> directoryOwnerOnlyPermission =
PosixFilePermissions.fromString("rwx------");

/** Additional data types not covered by standard JDBC */
public static final int EXTRA_TYPES_TIMESTAMP_LTZ = 50000;

Expand Down Expand Up @@ -914,51 +904,6 @@ public static Map<String, String> createCaseInsensitiveMap(Header[] headers) {
}
}

/** create a directory with Owner only permission (0600) */
@SnowflakeJdbcInternalApi
public static boolean createOwnerOnlyPermissionDir(String location) {
if (isWindows()) {
return false;
}

boolean isDirCreated = true;
Path dir = Paths.get(location);
try {
Files.createDirectory(
dir, PosixFilePermissions.asFileAttribute(directoryOwnerOnlyPermission));
} catch (IOException e) {
logger.error(
"Failed to set OwnerOnly permission for {}. This may cause the file download to fail ",
location);
isDirCreated = false;
}
return isDirCreated;
}

@SnowflakeJdbcInternalApi
public static void assureOnlyUserAccessibleFilePermissions(File file) throws IOException {
if (isWindows()) {
return;
}
boolean disableUserPermissions =
file.setReadable(false, false)
&& file.setWritable(false, false)
&& file.setExecutable(false, false);
boolean setOwnerPermissionsOnly = file.setReadable(true, true) && file.setWritable(true, true);

if (disableUserPermissions && setOwnerPermissionsOnly) {
logger.info("Successfuly set OwnerOnly permission for {}. ", file.getAbsolutePath());
} else {
file.delete();
logger.error(
"Failed to set OwnerOnly permission for {}. Failed to download", file.getAbsolutePath());
throw new IOException(
String.format(
"Failed to set OwnerOnly permission for %s. Failed to download",
file.getAbsolutePath()));
}
}

/**
* Check whether the OS is Windows
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ public void download(
transferOptions.setConcurrentRequestCount(parallelism);

blob.downloadToFile(localFilePath, null, transferOptions, opContext);
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(localFile);
stopwatch.stop();
long downloadMillis = stopwatch.elapsedMillis();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,6 @@ public void download(
outStream.flush();
outStream.close();
bodyStream.close();
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(localFile);
if (isEncrypting()) {
Map<String, String> userDefinedHeaders =
createCaseInsensitiveMap(response.getAllHeaders());
Expand Down Expand Up @@ -352,7 +351,6 @@ public void download(
logger.debug("Starting download without presigned URL", false);
blob.downloadTo(
localFile.toPath(), Blob.BlobSourceOption.shouldReturnRawInputStream(true));
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(localFile);
stopwatch.stop();
downloadMillis = stopwatch.elapsedMillis();
logger.debug("Download successful", false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,6 @@ public ExecutorService newExecutor() {
String iv = metaMap.get(AMZ_IV);

myDownload.waitForCompletion();
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(localFile);
stopwatch.stop();
long downloadMillis = stopwatch.elapsedMillis();

Expand Down
38 changes: 0 additions & 38 deletions src/test/java/net/snowflake/client/jdbc/SnowflakeUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package net.snowflake.client.jdbc;

import static net.snowflake.client.jdbc.SnowflakeUtil.createCaseInsensitiveMap;
import static net.snowflake.client.jdbc.SnowflakeUtil.createOwnerOnlyPermissionDir;
import static net.snowflake.client.jdbc.SnowflakeUtil.extractColumnMetadata;
import static net.snowflake.client.jdbc.SnowflakeUtil.getSnowflakeType;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -16,28 +15,18 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributes;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.sql.Types;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import net.snowflake.client.annotations.DontRunOnWindows;
import net.snowflake.client.category.TestTags;
import net.snowflake.client.core.ObjectMapperFactory;
import org.apache.http.Header;
import org.apache.http.message.BasicHeader;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

@Tag(TestTags.CORE)
public class SnowflakeUtilTest extends BaseJDBCTest {
Expand Down Expand Up @@ -128,33 +117,6 @@ public void shouldConvertHeadersCreateCaseInsensitiveMap() {
assertEquals("value2", map.get("Key2"));
}

@Test
@DontRunOnWindows
public void testCreateOwnerOnlyPermissionDir(@TempDir Path tempDir)
throws IOException, UnsupportedOperationException {
Path tmp = tempDir.resolve("folder-permission-testing");
String folderPath = tmp.toString();
boolean isDirCreated = createOwnerOnlyPermissionDir(folderPath);
assertTrue(isDirCreated);
assertTrue(tmp.toFile().isDirectory());
PosixFileAttributes attributes = Files.readAttributes(tmp, PosixFileAttributes.class);
Set<PosixFilePermission> permissions = attributes.permissions();
assertEquals(PosixFilePermissions.toString(permissions), "rwx------");
}

@Test
@DontRunOnWindows
public void testValidateFilePermission(@TempDir Path tempDir)
throws IOException, UnsupportedOperationException {
Path tmp = tempDir.resolve("fileTesting.txt");
File file = tmp.toFile();
assertTrue(file.createNewFile());
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(file);
PosixFileAttributes attributes = Files.readAttributes(tmp, PosixFileAttributes.class);
Set<PosixFilePermission> permissions = attributes.permissions();
assertEquals(PosixFilePermissions.toString(permissions), "rw-------");
}

private static SnowflakeColumnMetadata createExpectedMetadata(
JsonNode rootNode, JsonNode fieldOne, JsonNode fieldTwo) throws SnowflakeSQLLoggedException {
ColumnTypeInfo columnTypeInfo =
Expand Down

0 comments on commit 88bf88a

Please sign in to comment.