Skip to content

Commit e3349a3

Browse files
SNOW-2161716: Raise error if the config file is writable by others (#2501)
Co-authored-by: Patryk Czajka <[email protected]>
1 parent ae2bf2e commit e3349a3

File tree

3 files changed

+69
-1
lines changed

3 files changed

+69
-1
lines changed

DESCRIPTION.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ Source code is also available at: https://github.com/snowflakedb/snowflake-conne
1616
- Added an option to exclude `botocore` and `boto3` dependencies by setting `SNOWFLAKE_NO_BOTO` environment variable during installation
1717
- Revert changing exception type in case of token expired scenario for `Oauth` authenticator back to `DatabaseError`
1818
- Added support for pandas conversion for Day-time and Year-Month Interval types
19+
- Enhanced configuration file security checks with stricter permission validation.
20+
- Configuration files writable by group or others now raise a `ConfigSourceError` with detailed permission information, preventing potential credential tampering.
1921
- Fix "No AWS region was found" error if AWS region was set in `AWS_DEFAULT_REGION` variable instead of `AWS_REGION` for `WORKLOAD_IDENTITY` authenticator
2022
- Add `ocsp_root_certs_dict_lock_timeout` connection parameter to set the timeout (in seconds) for acquiring the lock on the OCSP root certs dictionary. Default value for this parameter is -1 which indicates no timeout.
2123

src/snowflake/connector/config_manager.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727

2828
LOGGER = logging.getLogger(__name__)
2929
READABLE_BY_OTHERS = stat.S_IRGRP | stat.S_IROTH
30-
30+
WRITABLE_BY_OTHERS = stat.S_IWGRP | stat.S_IWOTH
3131

3232
SKIP_WARNING_ENV_VAR = "SF_SKIP_WARNING_FOR_READ_PERMISSIONS_ON_CONFIG_FILE"
3333

@@ -337,6 +337,18 @@ def read_config(
337337
)
338338
continue
339339

340+
# Check for writable by others - this should raise an error
341+
if (
342+
not IS_WINDOWS # Skip checking on Windows
343+
and sliceoptions.check_permissions # Skip checking if this file couldn't hold sensitive information
344+
and filep.stat().st_mode & WRITABLE_BY_OTHERS != 0
345+
):
346+
file_stat = filep.stat()
347+
file_permissions = oct(file_stat.st_mode)[-3:]
348+
raise ConfigSourceError(
349+
f"file '{str(filep)}' is writable by group or others — this poses a security risk because it allows unauthorized users to modify sensitive settings. Your Permission: {file_permissions}"
350+
)
351+
340352
# Check for readable by others or wrong ownership - this should warn
341353
if (
342354
not IS_WINDOWS # Skip checking on Windows

test/unit/test_configmanager.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,60 @@ def test_log_debug_config_file_parent_dir_permissions(tmp_path, caplog):
639639
shutil.rmtree(tmp_dir)
640640

641641

642+
@pytest.mark.skipif(IS_WINDOWS, reason="chmod doesn't work on Windows")
643+
def test_error_config_file_writable_by_others(tmp_path):
644+
c_file = tmp_path / "config.toml"
645+
c1 = ConfigManager(file_path=c_file, name="root_parser")
646+
c1.add_option(name="b", parse_str=lambda e: e.lower() == "true")
647+
c_file.write_text(
648+
dedent(
649+
"""\
650+
b = true
651+
"""
652+
)
653+
)
654+
# Make file writable by others
655+
c_file.chmod(stat.S_IRUSR | stat.S_IWUSR | stat.S_IWOTH)
656+
file_permissions = oct(c_file.stat().st_mode)[-3:]
657+
658+
with pytest.raises(
659+
ConfigSourceError,
660+
match=re.escape(
661+
f"file '{str(c_file)}' is writable by group or others — this poses a security risk because it allows unauthorized users to modify sensitive settings. Your Permission: {file_permissions}"
662+
),
663+
):
664+
c1["b"]
665+
666+
667+
@pytest.mark.skipif(IS_WINDOWS, reason="chmod doesn't work on Windows")
668+
def test_error_config_file_writable_by_group(tmp_path):
669+
c_file = tmp_path / "config.toml"
670+
c1 = ConfigManager(file_path=c_file, name="root_parser")
671+
c1.add_option(name="b", parse_str=lambda e: e.lower() == "true")
672+
c_file.write_text(
673+
dedent(
674+
"""\
675+
b = true
676+
"""
677+
)
678+
)
679+
# Make file writable by group
680+
c_file.chmod(stat.S_IRUSR | stat.S_IWUSR | stat.S_IWGRP)
681+
file_permissions = oct(c_file.stat().st_mode)[-3:]
682+
683+
with pytest.raises(
684+
ConfigSourceError,
685+
match=re.escape(
686+
f"file '{str(c_file)}' is writable by group or others — this poses a security risk because it allows unauthorized users to modify sensitive settings. Your Permission: {file_permissions}"
687+
),
688+
):
689+
c1["b"]
690+
691+
# file permissions check can be skipped with unsafe_skip_file_permissions_check flag
692+
c1.read_config(skip_file_permissions_check=True)
693+
assert c1["b"] is True
694+
695+
642696
@pytest.mark.skipif(IS_WINDOWS, reason="chmod doesn't work on Windows")
643697
def test_skip_warning_config_file_permissions(tmp_path, monkeypatch):
644698
c_file = tmp_path / "config.toml"

0 commit comments

Comments
 (0)