Skip to content

Commit d703bd7

Browse files
hramezaniclaudeCopilot
authored
Prepare release 2.14.2 (#890)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent e95c30b commit d703bd7

3 files changed

Lines changed: 214 additions & 9 deletions

File tree

pydantic_settings/sources/providers/nested_secrets.py

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import os
22
import warnings
3+
from collections.abc import Iterator
34
from functools import reduce
4-
from glob import iglob
55
from pathlib import Path
66
from typing import TYPE_CHECKING, Any, Literal, Optional
77

@@ -146,17 +146,61 @@ def validate_secrets_path(self, path: Path) -> None:
146146
else:
147147
if not path.is_dir():
148148
raise SettingsError(f'secrets_dir must reference a directory, not a {path_type_label(path)}')
149-
secrets_dir_size = sum(f.stat().st_size for f in path.glob('**/*') if f.is_file())
149+
secrets_dir_size = sum(f.stat().st_size for f in self._iter_secret_files(path))
150150
if secrets_dir_size > self.secrets_dir_max_size:
151151
raise SettingsError(f'secrets_dir size is above {self.secrets_dir_max_size} bytes')
152152

153153
@staticmethod
154-
def load_secrets(path: Path) -> dict[str, str]:
155-
return {
156-
str(p.relative_to(path)): p.read_text().strip()
157-
for p in map(Path, iglob(f'{path}/**/*', recursive=True))
158-
if p.is_file()
159-
}
154+
def _iter_secret_files(path: Path) -> Iterator[Path]:
155+
"""Yield the secret files contained in ``path``.
156+
157+
``path`` is expected to already be resolved. The directory tree is walked
158+
explicitly so that symbolic links are handled safely:
159+
160+
* a file is only yielded if its real location stays within ``path``; entries
161+
that resolve outside of it (e.g. through a symbolic link) are skipped, so
162+
they neither contribute to the ``secrets_dir_max_size`` accounting nor get
163+
loaded;
164+
* each real directory is visited at most once, so cyclic or repeated
165+
symlinks cannot make the walk loop and inflate the size accounting or the
166+
number of loaded secrets.
167+
168+
Because the size check and the loader share this iterator, they always see
169+
the same set of files.
170+
"""
171+
seen_dirs: set[Path] = set()
172+
173+
def walk(directory: Path) -> Iterator[Path]:
174+
# Guard against symlink loops / a directory reachable through multiple
175+
# links being traversed more than once.
176+
resolved_dir = directory.resolve()
177+
if resolved_dir in seen_dirs:
178+
return
179+
seen_dirs.add(resolved_dir)
180+
try:
181+
entries = sorted(directory.iterdir())
182+
except OSError:
183+
return
184+
for entry in entries:
185+
resolved = entry.resolve()
186+
if resolved.is_dir():
187+
# Only descend into directories that stay within secrets_dir.
188+
# A symlinked directory pointing outside of ``path`` is not
189+
# followed at all, so we never walk (potentially large) external
190+
# trees and never read files from outside secrets_dir.
191+
if resolved == path or path in resolved.parents:
192+
yield from walk(entry)
193+
elif resolved.is_file() and path in resolved.parents:
194+
# Defense in depth: a file whose real location escapes
195+
# secrets_dir (e.g. a symlink pointing outside of ``path``) is
196+
# skipped from both the size accounting and the load.
197+
yield entry
198+
199+
yield from walk(path)
200+
201+
@classmethod
202+
def load_secrets(cls, path: Path) -> dict[str, str]:
203+
return {str(p.relative_to(path)): p.read_text().strip() for p in cls._iter_secret_files(path)}
160204

161205
def __repr__(self) -> str:
162206
return f'NestedSecretsSettingsSource(secrets_dir={self.secrets_dir!r})'

pydantic_settings/version.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
VERSION = '2.14.1'
1+
VERSION = '2.14.2'

tests/test_source_nested_secrets.py

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
from enum import Enum
22
from os import sep
3+
from pathlib import Path
4+
from unittest.mock import patch
35

46
import pytest
57
from pydantic import BaseModel
@@ -195,6 +197,165 @@ class Settings(AppSettings):
195197
}
196198

197199

200+
def test_symlink_dir_escaping_secrets_dir_is_ignored(env, tmp_path):
201+
"""Regression test for GHSA-4xgf-cpjx-pc3j.
202+
203+
A directory entry inside ``secrets_dir`` that is a symbolic link to a directory
204+
*outside* of ``secrets_dir`` must not be followed: its files must not leak into
205+
settings values.
206+
"""
207+
env.set('DB__USER', 'user')
208+
209+
secrets_dir = tmp_path / 'secrets'
210+
secrets_dir.mkdir()
211+
outside = tmp_path / 'outside'
212+
outside.mkdir()
213+
outside.joinpath('passwd').write_text('leaked-secret')
214+
215+
# secrets/db -> ../outside (escapes secrets_dir)
216+
secrets_dir.joinpath('db').symlink_to(outside)
217+
218+
class Settings(AppSettings):
219+
model_config = SettingsConfigDict(
220+
secrets_dir=secrets_dir,
221+
env_nested_delimiter='__',
222+
secrets_nested_subdir=True,
223+
)
224+
225+
# the out-of-tree file is not read; passwd keeps its default
226+
assert Settings().model_dump() == {
227+
'app_key': None,
228+
'db': {'user': 'user', 'passwd': None},
229+
}
230+
231+
232+
def test_symlink_file_escaping_secrets_dir_is_ignored(env, tmp_path):
233+
"""Regression test for GHSA-4xgf-cpjx-pc3j (file-level symlink variant)."""
234+
env.set('DB__USER', 'user')
235+
236+
secrets_dir = tmp_path / 'secrets'
237+
(secrets_dir / 'db').mkdir(parents=True)
238+
outside = tmp_path / 'outside'
239+
outside.mkdir()
240+
outside.joinpath('passwd').write_text('leaked-secret')
241+
242+
# secrets/db/passwd -> ../../outside/passwd (escapes secrets_dir)
243+
secrets_dir.joinpath('db', 'passwd').symlink_to(outside / 'passwd')
244+
245+
class Settings(AppSettings):
246+
model_config = SettingsConfigDict(
247+
secrets_dir=secrets_dir,
248+
env_nested_delimiter='__',
249+
secrets_nested_subdir=True,
250+
)
251+
252+
assert Settings().model_dump() == {
253+
'app_key': None,
254+
'db': {'user': 'user', 'passwd': None},
255+
}
256+
257+
258+
def test_symlink_escape_does_not_bypass_max_size(env, tmp_path):
259+
"""Regression test for GHSA-4xgf-cpjx-pc3j (size-limit bypass).
260+
261+
The ``secrets_dir_max_size`` accounting must see the same files as the loader.
262+
An out-of-tree file reached through a symlink previously counted as 0 bytes in
263+
the size check (``Path.glob``) while still being read by the loader
264+
(``glob.iglob(recursive=True)``). After the fix it is excluded from both, so the
265+
large out-of-tree file neither trips the size cap nor leaks into settings.
266+
"""
267+
env.set('DB__USER', 'user')
268+
269+
secrets_dir = tmp_path / 'secrets'
270+
secrets_dir.mkdir()
271+
outside = tmp_path / 'outside'
272+
outside.mkdir()
273+
# 512 bytes, well above the 100 byte cap below
274+
outside.joinpath('passwd').write_text('S' * 512)
275+
secrets_dir.joinpath('db').symlink_to(outside)
276+
277+
class Settings(AppSettings):
278+
model_config = SettingsConfigDict(
279+
secrets_dir=secrets_dir,
280+
env_nested_delimiter='__',
281+
secrets_nested_subdir=True,
282+
secrets_dir_max_size=100,
283+
)
284+
285+
# No SettingsError, and the out-of-tree payload is not loaded.
286+
assert Settings().model_dump() == {
287+
'app_key': None,
288+
'db': {'user': 'user', 'passwd': None},
289+
}
290+
291+
292+
def test_cyclic_symlink_does_not_inflate_size(env, tmp_path):
293+
"""Regression test for GHSA-4xgf-cpjx-pc3j (resource-consumption / CWE-400).
294+
295+
A cyclic (or repeated) symlink inside ``secrets_dir`` must not cause the
296+
directory walk to loop. Otherwise a single small file gets visited many times,
297+
inflating the ``secrets_dir_max_size`` accounting (and the number of loaded
298+
secrets). Each real directory must be traversed at most once.
299+
"""
300+
env.set('DB__USER', 'user')
301+
302+
secrets_dir = tmp_path / 'secrets'
303+
(secrets_dir / 'db').mkdir(parents=True)
304+
secrets_dir.joinpath('db', 'passwd').write_text('secret2')
305+
# secrets/db/loop -> secrets (cycle): a naive recursive glob would revisit
306+
# passwd dozens of times.
307+
secrets_dir.joinpath('db', 'loop').symlink_to(secrets_dir)
308+
309+
class Settings(AppSettings):
310+
model_config = SettingsConfigDict(
311+
secrets_dir=secrets_dir,
312+
env_nested_delimiter='__',
313+
secrets_nested_subdir=True,
314+
# passwd is 7 bytes; with the cycle un-guarded the walk would count it
315+
# many times and exceed this cap.
316+
secrets_dir_max_size=50,
317+
)
318+
319+
# The walk terminates, counts passwd once, and loads it exactly once.
320+
assert Settings().model_dump() == {
321+
'app_key': None,
322+
'db': {'user': 'user', 'passwd': 'secret2'},
323+
}
324+
325+
326+
def test_symlinked_dir_escaping_secrets_dir_is_not_walked(tmp_path):
327+
"""A symlinked directory pointing outside ``secrets_dir`` must not be traversed.
328+
329+
Even though out-of-tree files are filtered out, descending into the external
330+
tree wastes I/O and contradicts the intent of not following symlinks outside
331+
``secrets_dir`` (potential DoS via a large external tree). The walk must not
332+
``iterdir`` anything outside of ``secrets_dir``.
333+
"""
334+
secrets_dir = tmp_path / 'secrets'
335+
secrets_dir.mkdir()
336+
secrets_dir.joinpath('legit').write_text('ok')
337+
338+
external = tmp_path / 'external'
339+
(external / 'deep').mkdir(parents=True)
340+
external.joinpath('deep', 'passwd').write_text('leaked')
341+
secrets_dir.joinpath('link').symlink_to(external)
342+
343+
resolved_secrets = secrets_dir.resolve()
344+
walked: list[Path] = []
345+
original_iterdir = Path.iterdir
346+
347+
def tracking_iterdir(self):
348+
walked.append(self.resolve())
349+
return original_iterdir(self)
350+
351+
with patch.object(Path, 'iterdir', tracking_iterdir):
352+
files = list(NestedSecretsSettingsSource._iter_secret_files(resolved_secrets))
353+
354+
# only the in-tree file is yielded, and nothing outside secrets_dir is walked
355+
assert [f.name for f in files] == ['legit']
356+
assert all(d == resolved_secrets or resolved_secrets in d.parents for d in walked), walked
357+
358+
198359
@pytest.mark.parametrize(
199360
'conf,secrets,dirs,expected',
200361
(

0 commit comments

Comments
 (0)