fix(security): deny server secrets/DB within the local-file-access boundary

restrict_local_file_access confined reads to config_dir, but config_dir IS
the storage dir AND holds the server-managed secrets as siblings of the
per-flow upload subdirs: secret_key (Fernet master key), private_key.pem /
public_key.pem (JWT signing keys), and the SQLite DB (save_db_in_config_dir).
A tenant File-component input of "<flow>/../secret_key" routed through
build_full_path (no '..' check) resolved back to <config_dir>/secret_key,
passed the is_relative_to(config_dir) boundary, and was read -- disclosing the
key that decrypts every tenant's stored credentials. The control thus failed
its own stated goal under the very multi-tenant mode it adds.

enforce_local_file_access now denies the exact config_dir locations of
secret_key / private_key.pem / public_key.pem and the sqlite DB derived from
database_url (including the -wal / -shm / -journal sidecars that hold the same
row data, and the async sqlite+aiosqlite:/// + ?query URL forms), even though
they resolve inside the boundary. Matched at exact location only, not by
basename, so a tenant upload that merely shares a reserved name inside a flow
subdir stays readable. Fails safe on non-sqlite / empty / unavailable settings.

Known limitation (tracked separately): this does not scope reads per tenant,
so cross-tenant reads of <config_dir>/<other_flow_id>/<file> uploads remain
possible and need per-user/per-flow scoping in a follow-up.

Tests: reserved file blocked, traversal-to-reserved blocked, DB + WAL/SHM/
journal sidecars blocked, async+query URL blocked, same-named upload in a flow
subdir still allowed.
This commit is contained in:
Jordan Frazier
2026-06-07 20:49:25 -04:00
parent 1707415a85
commit 48c2e19d23
2 changed files with 133 additions and 1 deletions

View File

@ -8,10 +8,24 @@ When ``LANGFLOW_RESTRICT_LOCAL_FILE_ACCESS`` is enabled, resolved local file pat
within the storage data directory (``settings.config_dir``), where uploads live. The check is
a no-op when the setting is disabled (OSS default), so single-tenant deployments keep the
existing "read any local file by absolute path" behavior.
Reserved-secret denial: the storage data directory IS ``config_dir``, which also holds the
server-managed secret files as siblings of the per-flow upload subdirectories — the Fernet
master key (``secret_key``), the JWT signing keys (``private_key.pem`` / ``public_key.pem``),
and the SQLite DB when ``save_db_in_config_dir`` is set. The storage-boundary check alone would
permit reading those (e.g. ``<flow>/../secret_key`` resolves back inside ``config_dir``), which
would defeat the control's purpose — reading ``secret_key`` discloses every tenant's stored
credentials. So those exact files are denied explicitly even though they sit inside the boundary.
KNOWN LIMITATION (tracked for follow-up, see ``.claude/security-audit-findings.md``): this does
NOT scope reads per tenant — any ``<config_dir>/<other_flow_id>/<file>`` upload is still in
bounds, so one tenant can read another tenant's uploaded files. Closing that requires
per-user/per-flow scoping at the call sites and is deferred.
"""
from __future__ import annotations
import contextlib
from pathlib import Path
from lfx.logging import logger
@ -22,6 +36,13 @@ class LocalFileAccessError(ValueError):
"""Raised when a resolved path escapes the allowed storage root under restriction."""
# Server-managed secret/key file names that live directly under config_dir (see auth.py:
# ``secret_key``, ``private_key.pem``, ``public_key.pem``). Matched only at their exact
# config_dir location, never by basename — a tenant upload happens to be named "secret_key"
# inside a flow subdir is a different path and stays readable.
_RESERVED_SECRET_FILENAMES = frozenset({"secret_key", "private_key.pem", "public_key.pem"})
def is_local_file_access_restricted() -> bool:
"""Return True if local file access is restricted to the storage directory."""
try:
@ -34,6 +55,36 @@ def is_local_file_access_restricted() -> bool:
return False
def _reserved_secret_paths(data_dir: Path) -> set[Path]:
"""Resolved paths of server-managed secret/key/DB files under the storage dir.
Reading any of these would compromise the deployment (the Fernet master key decrypts every
tenant's credentials; the ``*.pem`` keys allow auth-token forgery; the SQLite DB holds all
rows), so they are denied even though they resolve inside the containment boundary.
"""
reserved = {(data_dir / name).resolve() for name in _RESERVED_SECRET_FILENAMES}
# Add the SQLite DB file when it lives under config_dir (``save_db_in_config_dir``).
# database_url is assembled as ``sqlite:///<absolute path>`` (see settings/base.py); the
# async ``sqlite+aiosqlite:///`` form is also covered by the ``sqlite`` prefix.
try:
db_url = get_settings_service().settings.database_url or ""
except Exception: # noqa: BLE001 - settings may be unavailable; nothing to add
db_url = ""
if db_url.startswith("sqlite") and ":///" in db_url:
# Drop any ``?query`` so a custom LANGFLOW_DATABASE_URL still resolves to the file.
db_path_str = db_url.split(":///", 1)[1].split("?", 1)[0]
if db_path_str:
with contextlib.suppress(OSError):
db_path = Path(db_path_str).resolve()
reserved.add(db_path)
# WAL/SHM/journal sidecars hold un-checkpointed DB pages (the same row data),
# so they must be denied alongside the main DB file.
for suffix in ("-wal", "-shm", "-journal"):
reserved.add(Path(str(db_path) + suffix))
return reserved
def enforce_local_file_access(resolved_path: str | Path) -> Path:
"""Ensure a resolved local path is inside the storage data dir when restriction is on.
@ -68,4 +119,14 @@ def enforce_local_file_access(resolved_path: str | Path) -> Path:
"(LANGFLOW_RESTRICT_LOCAL_FILE_ACCESS=true). Use an uploaded file instead."
)
raise LocalFileAccessError(msg)
# The storage dir is config_dir, which also holds server-managed secret/key/DB files as
# siblings of the upload subdirs; deny those explicitly (a traversal like "<flow>/../secret_key"
# resolves back inside the boundary but must not be readable).
if candidate in _reserved_secret_paths(data_dir):
msg = (
"Access to this server-managed file is not permitted "
"(LANGFLOW_RESTRICT_LOCAL_FILE_ACCESS=true)."
)
raise LocalFileAccessError(msg)
return path

View File

@ -13,11 +13,13 @@ from lfx.utils.file_path_security import (
@contextmanager
def mock_settings(*, restricted: bool, config_dir: str):
def mock_settings(*, restricted: bool, config_dir: str, database_url: str = ""):
with patch("lfx.utils.file_path_security.get_settings_service") as mock_get:
settings = MagicMock()
settings.settings.restrict_local_file_access = restricted
settings.settings.config_dir = config_dir
# Explicit string so the reserved-DB derivation in _reserved_secret_paths is deterministic.
settings.settings.database_url = database_url
mock_get.return_value = settings
yield
@ -91,3 +93,72 @@ def test_symlink_inside_storage_pointing_inside_allowed(tmp_path):
link.symlink_to(real)
with mock_settings(restricted=True, config_dir=str(storage)):
assert enforce_local_file_access(str(link)) == Path(str(link))
@pytest.mark.parametrize("name", ["secret_key", "private_key.pem", "public_key.pem"])
def test_reserved_secret_file_blocked(tmp_path, name):
"""The server-managed secret/key files in config_dir are denied even though they sit inside it."""
(tmp_path / name).write_text("SENSITIVE")
with mock_settings(restricted=True, config_dir=str(tmp_path)), pytest.raises(LocalFileAccessError):
enforce_local_file_access(str(tmp_path / name))
def test_reserved_secret_file_via_traversal_blocked(tmp_path):
"""A traversal that resolves back to a reserved secret file is denied.
This is the actual exploit shape: a storage-path input like "<flow>/../secret_key" routes
through build_full_path (no '..' check) to <config_dir>/<flow>/../secret_key, which resolves
back inside the boundary.
"""
(tmp_path / "secret_key").write_text("MASTER KEY")
traversal = str(tmp_path / "some-flow" / ".." / "secret_key")
with mock_settings(restricted=True, config_dir=str(tmp_path)), pytest.raises(LocalFileAccessError):
enforce_local_file_access(traversal)
def test_reserved_db_file_blocked(tmp_path):
"""The SQLite DB under config_dir (save_db_in_config_dir) is denied."""
db = tmp_path / "langflow.db"
db.write_text("db")
with (
mock_settings(restricted=True, config_dir=str(tmp_path), database_url=f"sqlite:///{db}"),
pytest.raises(LocalFileAccessError),
):
enforce_local_file_access(str(db))
@pytest.mark.parametrize("suffix", ["-wal", "-shm", "-journal"])
def test_reserved_db_sidecar_blocked(tmp_path, suffix):
"""SQLite WAL/SHM/journal sidecars hold un-checkpointed DB pages and are denied too."""
db = tmp_path / "langflow.db"
sidecar = tmp_path / f"langflow.db{suffix}"
sidecar.write_text("pages")
with (
mock_settings(restricted=True, config_dir=str(tmp_path), database_url=f"sqlite:///{db}"),
pytest.raises(LocalFileAccessError),
):
enforce_local_file_access(str(sidecar))
def test_reserved_db_with_async_driver_and_query_blocked(tmp_path):
"""An async sqlite URL with a query string still resolves to the protected DB file."""
db = tmp_path / "langflow.db"
db.write_text("db")
url = f"sqlite+aiosqlite:///{db}?check_same_thread=false"
with (
mock_settings(restricted=True, config_dir=str(tmp_path), database_url=url),
pytest.raises(LocalFileAccessError),
):
enforce_local_file_access(str(db))
def test_upload_named_like_secret_in_flow_subdir_allowed(tmp_path):
"""A tenant upload that merely shares a reserved name but lives in a flow subdir stays readable.
Proves the denial matches the exact config_dir location, not the basename anywhere.
"""
upload = tmp_path / "flow-id" / "secret_key"
upload.parent.mkdir(parents=True)
upload.write_text("just a user file named secret_key")
with mock_settings(restricted=True, config_dir=str(tmp_path)):
assert enforce_local_file_access(str(upload)) == Path(str(upload))