fix(project): stop process-lifetime lru_cache from serving stale project state - #1124
Conversation
…ect state (VUL-603) Two separate bugs in project discovery/parsing, both caused by caching on the wrong key: - find_project_root(None) cached on the literal None key, so cwd was only read inside the function body on the first call. A chdir mid-process (long-lived orchestrators, notebooks, CI workers) got the previous project's root back. Demonstrated directly: two projects each define an app of the same name with different secrets/auth, and the second project's lookup returned the first project's secrets. - _load_toml(path) was keyed on path alone, so an in-place edit within a live process was invisible. Fix: the srcs=None case in find_project_root is no longer cached on a constant key, cwd is read fresh every call. _load_toml is now keyed on (path, mtime_ns) with an explicit maxsize since lru_cache never evicts on its own. _cached_resolve is untouched, it is genuinely path-pure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 72b861d. Configure here.
|
|
||
|
|
||
| @lru_cache | ||
| def _find_project_root_cached(srcs: Tuple[str, ...]) -> Tuple[Path, str]: |
There was a problem hiding this comment.
Relative srcs still cache cwd-stale
Medium Severity
The non-None branch assigns resolved_srcs = tuple(srcs) without resolving against cwd, so relative entries become cache keys while _find_project_root_cached still joins them with Path.cwd(). A later chdir with the same relative srcs can reuse the prior root. That breaks the stated invariant that the cached helper only sees resolved paths, and tuple(srcs) newly enables caching for list arguments that used to be unhashable.
Reviewed by Cursor Bugbot for commit 72b861d. Configure here.
| project root was discovered. | ||
| """ | ||
| if not srcs: | ||
| srcs = [str(_cached_resolve(Path.cwd()))] | ||
| # cwd-dependent, so this case cannot be cached on a constant key | ||
| # (see _find_project_root_cached, which only ever sees resolved srcs) | ||
| resolved_srcs: Tuple[str, ...] = (str(_cached_resolve(Path.cwd())),) | ||
| else: | ||
| resolved_srcs = tuple(srcs) | ||
|
|
||
| return _find_project_root_cached(resolved_srcs) | ||
|
|
||
|
|
||
| @lru_cache | ||
| def _find_project_root_cached(srcs: Tuple[str, ...]) -> Tuple[Path, str]: | ||
| path_srcs = [_cached_resolve(Path(Path.cwd(), src)) for src in srcs] | ||
|
|
||
| # A list of lists of parents for each 'src'. 'src' is included as a |
There was a problem hiding this comment.
🟡 The new caching fix only resolves cwd-blindness for the srcs=None branch of find_project_root. When explicit (relative) srcs are passed, resolved_srcs = tuple(srcs) skips cwd resolution and is cached via _find_project_root_cached's @lru_cache, which is keyed only on that raw tuple — even though the cached function's body still reads Path.cwd() to resolve relative srcs. That reproduces the exact cwd-blind staleness this PR is fixing, just for the non-None branch. It's currently latent since every in-repo caller (find_pyproject_toml, cli/_utils.py) passes None, but worth closing (resolve each src against cwd in find_project_root before caching, mirroring the None branch) given the PR explicitly targets eliminating cwd-blind caching.
Extended reasoning...
The gap: find_project_root (projects/fal/src/fal/project.py:38-54) has two branches. The if not srcs: branch now correctly resolves Path.cwd() before delegating to the cached function, which is the whole point of this PR. But the else branch just does resolved_srcs = tuple(srcs) — no cwd resolution — and hands that raw tuple straight to _find_project_root_cached, an @lru_cache-decorated function keyed solely on srcs.
Inside _find_project_root_cached, the very first line is path_srcs = [_cached_resolve(Path(Path.cwd(), src)) for src in srcs] — it still reads live Path.cwd() to resolve each relative src into an absolute path. Since cwd is read inside the cached function but is not part of the cache key, the memoized result silently goes stale exactly the way this PR set out to prevent.
Concrete repro: Create two temp projects, each with its own pyproject.toml (containing [tool.fal]) and a sub/ subdirectory. From project A's directory, call find_project_root(["sub"]) — it resolves sub against A's cwd, walks up, and finds A's root, caching the result under key ("sub",). Now chdir into project B and call find_project_root(["sub"]) again with the identical relative arg. Because the cache key ("sub",) is unchanged, _find_project_root_cached returns the memoized tuple for A's root instead of re-resolving against B's new cwd — B's actual project root is never discovered.
Why nothing catches this today: the docstring comment added by this PR (# ...see _find_project_root_cached, which only ever sees resolved srcs) is not accurate — the explicit-srcs branch passes through raw, possibly-relative srcs unresolved. It's also true that this specific behavior isn't new: pre-PR, the entire find_project_root was itself @lru_cached with the same latent relative-srcs blindness, so this PR doesn't regress anything that previously worked. Absolute explicit srcs are unaffected (Path(cwd, abs_src) ignores cwd entirely, so the key is already complete for that case).
Impact: currently none in this repo. Both in-repo callers of find_project_root — find_pyproject_toml and cli/_utils.py:99 — always call it with None, never with an explicit relative srcs list, so this path is unexercised. But find_project_root is part of the module's public surface, and the PR's own description frames long-lived, multi-project processes (CI workers, notebooks, SDK-wrapping orchestrators) as exactly the threat model being closed. Any future or external caller that passes a relative srcs list across a chdir boundary in such a process would hit the identical bug this PR is supposed to eliminate.
Suggested fix: mirror the None branch — resolve each element of srcs against Path.cwd() into an absolute path before calling _find_project_root_cached, e.g. resolved_srcs = tuple(str(_cached_resolve(Path(Path.cwd(), s))) for s in srcs). That makes the cache key fully determine the result regardless of cwd, consistent with the invariant the docstring comment already (incorrectly) claims holds.


Summary
Fixes VUL-603. Two separate bugs, both from caching on the wrong key, kept
apart in the commit and here for the same reason.
Bug 1, cwd-blind discovery (the security-relevant one):
find_project_root(None)cached on the literalNonekey. cwd was onlyread inside the function body, so a
chdirmid-process returned theprevious project's root. Long-lived processes that deploy more than one
project (CI workers, notebooks, orchestrators wrapping the SDK) hit this;
fal deployfrom a shell does not, each invocation is a fresh process.Demonstrated directly, not just at the root-discovery level: two projects
each define an app of the same name with different
secrets/auth. Afterloading project A's app,
chdirto project B and requesting thesame-named app returned project A's
secrets/authinstead of projectB's own.
Bug 2, mtime-blind loading:
_load_toml(path)was keyed on path alone,so an in-place edit within a live process was invisible.
Fix
find_project_root: thesrcs=Nonecase is no longer cached on aconstant key. cwd is resolved fresh on every call; the actual cached work
moved to
_find_project_root_cached(srcs: Tuple[str, ...])._load_toml: now keyed on(path, mtime_ns)via_load_toml_cached,with an explicit
maxsize=128sincelru_cachenever evicts on its own._cached_resolveis untouched. It is genuinely path-pure, no cwddependence.
No public
clear_cache()added. That would put the burden on every callerto know about a bug they cannot see.
Test plan
tests/unit/test_project.py:find_pyproject_tomlreturns the rightroot after a
chdirwithin one process;parse_pyproject_tomlobserves an in-place edit within one process (mtime forced forward to
cover coarse filesystem mtime resolution)
tests/unit/cli/test_utils.py:get_app_data_from_tomldoes notleak project A's secrets/auth to project B after a
chdir. This isthe regression test that encodes the actual security property.
tests/unitsuite: 885 passed, 3 skipped (hardware-dependent),6 pre-existing failures in
test_deploy.py, same ones reproducedidentically on unmodified
main, unrelated to this diff (also seenin fix(cli): strict validation for [tool.fal] pyproject manifest #1123)
ruff check/ruff format --checkclean🤖 Generated with Claude Code
Note
High Risk
Corrects a security-relevant cross-project config/secrets leak in long-lived SDK/CLI usage; behavior change is intentional but touches auth and secret resolution paths.
Overview
Fixes stale project state in long-lived processes caused by
lru_cachekeys that ignored cwd and file mtime.Project discovery:
find_project_root(None)no longer caches on a constantNonekey. Each call resolves cwd first, then delegates to new_find_project_root_cachedkeyed on resolved source paths—so a mid-processchdirpicks up the correctpyproject.toml/ root instead of the first project touched.TOML loading:
_load_tomlnow includesmtime_nsin the cache key via_load_toml_cached(maxsize=128), so in-place edits to the same path are re-read instead of returning an old parse.Tests: Unit coverage for
chdiracross two projects, mtime-aware re-parse, and a CLI regression ensuring same-named apps in different projects do not leak secrets / auth after switching directories.Reviewed by Cursor Bugbot for commit 72b861d. Bugbot is set up for automated code reviews on this repo. Configure here.