From bbd5aa10fcdb7fd49e4cd3118022b1b88d43adf3 Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 14:21:01 +0200 Subject: [PATCH 1/9] fix(loader): confine $ref to the contract's directory tree A relative $ref was joined to the contract's directory with '../' honoured, and the only guards were "not absolute" and the system- directory deny list. So a contract could compose any dict-rooted YAML/JSON file the process can read into itself, and `fluid bundle` printed it back (`fluid validate` passed). The Command Center runs these commands on contracts its users upload. Every external $ref now goes through one check, fluid_build/util/ref_confinement.py::confine_ref: - a URL scheme (file:// included) or //host form is treated as remote and refused; nothing in FLUID fetches remote refs - an absolute path (POSIX, Windows drive, UNC, rooted) is refused - the target is Path.resolve()-d ('..' and symlinks) and must be is_relative_to(root), root resolved the same way - the root is the root contract's directory; nested refs are held to that same root, not to the fragment's directory - the check runs before the target is tested for existence, so a refused ref is not a file-existence probe The error is RefConfinementError, a RefResolutionError subclass (so existing handlers catch it), naming the ref, the JSON pointer of the $ref node and the file it is in, with .ref/.pointer/.source/.root. Widening the root is an explicit opt-in: ref_root= on load_contract, load_with_overlay and compile_contract, or FLUID_REF_ROOT for the CLI. It must be an existing directory containing the contract, a blank value means the default, and it is consulted only when the contract has an external ref. URLs stay refused and the SecurePathValidator deny list stays as a second layer under a widened root. The bundle validator hands sources/openapi/* to openapi-spec-validator, whose default handlers follow file:// (the target's values surface in the error) and http(s)://. A bundled fragment has no directory, so the same check with no root allows only '#/...' refs; anything else is an OAS-REF-EXTERNAL error and the validator is not run on that fragment. Same-document '#/...' refs are unchanged. Four tests that pinned the old '../sibling' behaviour now go through the opt-in. Borrowed, not built: datamodel-code-generator >= 0.62.0's containment fix for CVE-2026-55389 (resolve, then is_relative_to(base_path); file:// treated as remote; widening is an explicit opt-in) and python-jsonschema/referencing's prefix-checked filesystem retriever raising NoSuchResource. Neither resolver composes YAML fragments with a JSON-pointer suffix, so the policy is borrowed and the resolver kept. Docs: docs/contract-refs.md; SECURITY.md and AGENTS.md invariants. --- AGENTS.md | 1 + SECURITY.md | 1 + docs/contract-refs.md | 152 +++++++++ fluid_build/forge/core/validators.py | 34 +- fluid_build/loader.py | 178 ++++++++-- fluid_build/util/ref_confinement.py | 221 ++++++++++++ tests/forge/test_openapi_external_refs.py | 110 ++++++ tests/test_loader_ref_confinement.py | 387 ++++++++++++++++++++++ tests/test_loader_refs.py | 27 +- tests/util/test_ref_confinement.py | 81 +++++ 10 files changed, 1158 insertions(+), 34 deletions(-) create mode 100644 docs/contract-refs.md create mode 100644 fluid_build/util/ref_confinement.py create mode 100644 tests/forge/test_openapi_external_refs.py create mode 100644 tests/test_loader_ref_confinement.py create mode 100644 tests/util/test_ref_confinement.py diff --git a/AGENTS.md b/AGENTS.md index f6989d96..5e395899 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -231,6 +231,7 @@ A 2026-04-16 security review (see `SECURITY_REVIEW.md` if committed, or the PR # - **Encrypted credential store fails loud.** `credentials/encrypted_store.py::_load_store` raises `CredentialError` on `InvalidToken` — never silently returns `{}` (which previously caused destructive overwrite on next write with the wrong key). - **Tool errors are typed, not text.** `dispatch_tool_call` returns `{"error": , "message": "Tool … failed — see server logs"}` on exception; the full `exc` goes to `LOG.warning(..., exc_info=True)` where the redactor can scrub it. Exception text is never round-tripped into the LLM context (prevents path / hostname / env-var leaks). - **Subprocess argv sanitisation.** `cli/auth.py::_sanitize_argv` redacts values of `--password`, `--token`, `--api-key`, `--key-file`, any flag ending in `-secret`/`-key`/`-token`/`-password`/`-passphrase`. Called from `AuthProvider._run_command` before DEBUG-logging the command. +- **`$ref` resolution is confined.** Every external `$ref` goes through `util/ref_confinement.py::confine_ref`: URL schemes (`file://` included) and absolute paths are refused, and the `Path.resolve()`-d target must be `is_relative_to` the ref root — the root contract's directory unless the caller passes `ref_root=` / sets `FLUID_REF_ROOT`. Nested refs are held to the same root. Bundled OpenAPI fragments have no root, so only `#/...` refs reach openapi-spec-validator. A new `$ref` resolver must call `confine_ref`, not re-implement it. --- diff --git a/SECURITY.md b/SECURITY.md index 1f999a96..473091ee 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -58,6 +58,7 @@ FLUID Forge includes several built-in security measures: - **Credential redaction** — secrets are redacted from logs and plan output - **Provider auth isolation** — each provider manages its own authentication boundary - **Policy-as-code** — governance rules compile to native cloud IAM before deployment +- **`$ref` confinement** — a contract's `$ref`s may only name files inside the contract's own directory tree (symlinks resolved); URLs and absolute paths are refused. Widening the root is an explicit opt-in (`FLUID_REF_ROOT` / `ref_root=`). See [docs/contract-refs.md](docs/contract-refs.md). ## Plugin Trust Model diff --git a/docs/contract-refs.md b/docs/contract-refs.md new file mode 100644 index 00000000..7cfc8472 --- /dev/null +++ b/docs/contract-refs.md @@ -0,0 +1,152 @@ +# Composing a contract from fragments with `$ref` + +A contract can pull any object from another YAML or JSON file with `$ref`. +`fluid validate`, `plan`, `apply` and `bundle` resolve every ref before they +do anything else, so the rest of the pipeline sees one document. + +```text +orders/ +├── contract.fluid.yaml +├── owner.yaml +└── fragments/ + ├── builds/ingest.yaml + └── policy.yaml +``` + +```yaml +# orders/contract.fluid.yaml +id: sales.orders_v1 +metadata: + owner: + $ref: ./owner.yaml # the whole file +builds: + - $ref: fragments/builds/ingest.yaml # refs work inside lists +exposes: + - exposeId: orders + policy: + $ref: fragments/policy.yaml#/gold # file + JSON pointer +``` + +```bash +fluid validate orders/contract.fluid.yaml # refs resolved transparently +fluid bundle orders/contract.fluid.yaml # print the single resolved document +``` + +A ref is resolved relative to the file that contains it, so +`fragments/builds/ingest.yaml` may itself say `$ref: ../policy.yaml`. + +A `$ref` node is an object whose only key is `$ref`. Same-document refs +(`$ref: "#/definitions/x"`) are left in place as written. + +--- + +## Where a ref may point: the ref root + +Every ref must name a file inside the **ref root**. By default the ref root is +the directory that holds the root contract file (`orders/` above), and it +applies to every ref, including refs inside fragments. A fragment in +`orders/fragments/` can reach anything under `orders/` and nothing outside it. + +The check runs after `..` segments and symlinks are resolved, so neither can +be used to get out: + +| `$ref` written in `orders/contract.fluid.yaml` | Result | +|---|---| +| `./owner.yaml`, `fragments/policy.yaml#/gold` | resolved | +| `../shared/policy.yaml` | refused: escapes the ref root | +| `./link.yaml` where `link.yaml` is a symlink to `/home/me/x.yaml` | refused: escapes the ref root | +| `/etc/hosts`, `C:\x.yaml`, `\\server\share\x.yaml` | refused: must be a relative path | +| `file:///…`, `https://…`, `s3://…`, `//host/…` | refused: remote refs are not supported | + +A refused ref fails the command with a typed error that names the ref, the +[JSON pointer](https://www.rfc-editor.org/rfc/rfc6901) of the `$ref` node, and +the file it was written in: + +```text +❌ Validation error: contract_load_failed + error: $ref '../shared/policy.yaml' at JSON pointer '/exposes/0/policy' in + /work/orders/contract.fluid.yaml escapes the ref root /work/orders (after + resolving '..' and symlinks); refs may only name files inside it. … +``` + +`fluid validate` exits 1 and `fluid bundle` exits 2. The check runs before the +target is opened, so the error is the same whether or not the target exists. + +**Why.** Contracts are often untrusted input: a platform such as the FLUID +Command Center runs `fluid validate` and `fluid bundle` on contracts its users +upload. Without the root, a contract could compose any YAML or JSON file the +process can read into itself, and `fluid bundle` would print it back. + +--- + +## Widening the root (monorepos) + +To share fragments between products, set the ref root to a directory that +contains both the contracts and the shared fragments: + +```text +repo/ +├── shared/policy.yaml +└── products/orders/contract.fluid.yaml # $ref: ../../shared/policy.yaml +``` + +```bash +FLUID_REF_ROOT=repo fluid validate repo/products/orders/contract.fluid.yaml +``` + +From Python, pass `ref_root=` to the loader: + +```python +from fluid_build.loader import load_contract, RefConfinementError + +contract = load_contract("repo/products/orders/contract.fluid.yaml", ref_root="repo") +``` + +`load_contract`, `load_with_overlay` and `compile_contract` all accept +`ref_root`. The argument wins over `FLUID_REF_ROOT`. + +Rules for the wider root: + +- It must be an existing directory that contains the root contract. If it + does not, the command fails and names the setting it came from. +- A blank `FLUID_REF_ROOT` counts as unset: the default root applies. +- It is only consulted when the contract has a ref to another file, so a + `FLUID_REF_ROOT` left in your shell does not affect other contracts. +- It widens the root and nothing else. URLs and absolute paths are still + refused, and refs that resolve into system directories (`/etc`, `/proc`, + `/private/etc` on macOS, …) are still blocked. + +Set it to the narrowest directory that works. Setting it to `/` turns the +confinement off. + +--- + +## Catching the error in Python + +`RefConfinementError` is a subclass of `RefResolutionError`, so existing +`except RefResolutionError` handlers catch it. It carries the details as +attributes: + +```python +from fluid_build.loader import RefConfinementError, load_contract + +try: + load_contract(path) +except RefConfinementError as err: + print(err.ref) # '../shared/policy.yaml' + print(err.pointer) # '/exposes/0/policy' + print(err.source) # file that contains the ref + print(err.root) # the ref root it escaped +``` + +--- + +## OpenAPI fragments inside a bundle + +When `fluid validate .tgz` checks the OpenAPI documents extracted into +`sources/openapi/`, the same check applies with **no** root, because a bundled +fragment has no directory. Only same-document refs +(`$ref: "#/components/schemas/Order"`) are allowed. Any other `$ref` is +reported as an `OAS-REF-EXTERNAL` error, and openapi-spec-validator is not +run on that fragment. If it were run, it would follow `file://` and +`http(s)://` refs. Inline the referenced schemas under `components` instead. diff --git a/fluid_build/forge/core/validators.py b/fluid_build/forge/core/validators.py index 7443c9a8..0a49f3d4 100644 --- a/fluid_build/forge/core/validators.py +++ b/fluid_build/forge/core/validators.py @@ -46,6 +46,11 @@ import yaml from fluid_build.forge.core.bundle import SOURCE_SENTINEL, validate_manifest +from fluid_build.util.ref_confinement import ( + RefConfinementError, + confine_ref, + iter_external_refs, +) from fluid_build.util.safe_yaml import load_yaml_safe LOG = logging.getLogger("fluid.forge.core.validators") @@ -253,6 +258,25 @@ def validate_sql( return issues +def _external_openapi_ref_issues(path: str, spec: Any) -> List[ValidationIssue]: + """One ``OAS-REF-EXTERNAL`` error per ``$ref`` that leaves the document.""" + issues: List[ValidationIssue] = [] + for pointer, ref in iter_external_refs(spec): + try: + confine_ref(ref, ref.split("#", 1)[0], base_dir=None, root=None, pointer=pointer) + except RefConfinementError as exc: + issues.append( + ValidationIssue( + file=path, + validator="openapi-spec-validator", + severity="error", + message=str(exc), + code="OAS-REF-EXTERNAL", + ) + ) + return issues + + def validate_openapi( path: str, content: bytes, @@ -294,7 +318,15 @@ def validate_openapi( ) ] - issues: List[ValidationIssue] = [] + # openapi-spec-validator follows external ``$ref``s through its default + # handlers — ``file://`` reads a host file (whose values then surface in + # the validation error) and ``http(s)://`` makes a request. A bundled + # fragment has no directory of its own, so the shared confinement check + # allows only same-document ``#/...`` refs; anything else is reported + # and the validator is not called on this fragment. + issues = _external_openapi_ref_issues(path, spec) + if issues: + return issues try: # validate_spec (legacy) and validate (current) both exist; prefer # whichever is available without importing an exact entry point. diff --git a/fluid_build/loader.py b/fluid_build/loader.py index 2417e3fc..61a71091 100644 --- a/fluid_build/loader.py +++ b/fluid_build/loader.py @@ -17,10 +17,20 @@ import json import logging +import os import threading from pathlib import Path from typing import Any, Dict, List, Mapping, Optional, Set, Tuple, Union +from fluid_build.util.ref_confinement import ( + REF_ROOT_ENV, + RefConfinementError, + RefResolutionError, + confine_ref, + format_pointer, + iter_external_refs, +) + try: import yaml # type: ignore except Exception: # pragma: no cover @@ -28,6 +38,9 @@ __all__ = [ + "REF_ROOT_ENV", + "RefConfinementError", + "RefResolutionError", "available_overlay_envs", "load_contract", "load_with_overlay", @@ -194,8 +207,16 @@ def _deep_merge(base: Dict[str, Any], overlay: Dict[str, Any]) -> Dict[str, Any] _MAX_REF_DEPTH = 20 # safety limit against accidental deep nesting -class RefResolutionError(Exception): - """Raised when a $ref cannot be resolved.""" +# ``RefResolutionError`` and its confinement subclass live in +# ``fluid_build.util.ref_confinement`` (stdlib-only, shared with the bundle +# OpenAPI validator) and are re-exported here, so +# ``from fluid_build.loader import RefResolutionError`` keeps working. + +_REF_ROOT_HINT = ( + f"To compose fragments from a wider tree (e.g. a monorepo's shared/ " + f"directory), set {REF_ROOT_ENV} to that directory or pass ref_root= to " + f"the loader; see docs/contract-refs.md." +) def _is_ref_node(obj: Any) -> bool: @@ -248,10 +269,20 @@ def _resolve_pointer(obj: Any, pointer: str) -> Any: return current +def _pointer_parts(pointer: Optional[str]) -> Tuple[str, ...]: + """Segments of a ``#/a/b`` fragment, for error locations.""" + if not pointer or pointer == "/": + return () + return tuple(pointer.strip("/").split("/")) + + def _resolve_refs( obj: Any, base_dir: Path, *, + ref_root: Optional[Path] = None, + _source: Optional[Path] = None, + _loc: Tuple[Union[str, int], ...] = (), _seen: Optional[Set[str]] = None, _depth: int = 0, ) -> Any: @@ -260,13 +291,22 @@ def _resolve_refs( Supports: - External file refs: ``$ref: ./path/to/file.yaml`` - File + pointer: ``$ref: ./file.yaml#/section`` - - Same-file pointer: ``$ref: "#/definitions/x"`` (not yet — reserved) + - Same-file pointer: ``$ref: "#/definitions/x"`` (left in place as-is) - Refs inside lists: ``builds: [{ $ref: ./builds/ingest.yaml }]`` Protections: + - Confinement: every external ref goes through + :func:`fluid_build.util.ref_confinement.confine_ref`. The target, + after ``..`` and symlinks are resolved, must sit inside ``ref_root`` + (default: ``base_dir`` of the first call, i.e. the root contract's + directory). Nested refs are held to the SAME root, not to the + directory of the fragment that contains them. URLs (``file://`` + included) and absolute paths are refused. + - System-directory deny list (``SecurePathValidator``) as a second + layer, for callers that widen ``ref_root``. - Circular reference detection (tracks resolved absolute paths) - Depth limit (``_MAX_REF_DEPTH``) to prevent runaway recursion - - Clear error messages with file paths for debugging + - Clear error messages naming the ref and its JSON pointer """ if _depth > _MAX_REF_DEPTH: raise RefResolutionError( @@ -276,6 +316,7 @@ def _resolve_refs( if _seen is None: _seen = set() + root = base_dir if ref_root is None else ref_root # ── Handle $ref node ────────────────────────────────────────── if _is_ref_node(obj): @@ -292,20 +333,24 @@ def _resolve_refs( LOG.debug("skipping_same_file_ref", extra={"ref": ref_value}) return obj - ref_path = (base_dir / file_part).resolve() - - # Security (F3): block absolute $ref paths and system directories. - # Relative refs (including ../sibling/) are allowed for monorepo - # layouts, but absolute paths like /etc/passwd are rejected. - if file_part.startswith("/"): - raise RefResolutionError(f"$ref must be a relative path, got absolute: {ref_value}") - # Block system directories even when reached via ``../`` traversal. - # The previous inline check used a hand-rolled, Linux-only prefix - # list — it missed macOS (``/etc`` resolves to ``/private/etc``) - # and Windows entirely. Route through the platform-aware + # Confinement: a URL, an absolute path, or a target outside the root + # is refused before the target is tested for existence or opened, so + # a refused ref cannot probe the host for files either. + ref_path = confine_ref( + ref_value, + file_part, + base_dir=base_dir, + root=root, + pointer=format_pointer(_loc), + source=_source, + root_hint=_REF_ROOT_HINT, + ) + + # Defense in depth (F3): system directories stay blocked even when a + # caller widens ``ref_root``. Route through the platform-aware # ``SecurePathValidator`` so the deny set matches the rest of the - # CLI. Imported lazily to keep ``loader.py``'s import graph free - # of the ``cli`` package. + # CLI (it knows macOS ``/etc`` is ``/private/etc``). Imported lazily + # to keep ``loader.py``'s import graph free of the ``cli`` package. try: from fluid_build.cli.core import FluidCLIError from fluid_build.cli.security import SecurePathValidator, get_security_context @@ -353,8 +398,17 @@ def _resolve_refs( f"Failed to resolve pointer '{pointer}' in '{ref_value}': {e}" ) from e - # Recursively resolve refs in the loaded content - result = _resolve_refs(resolved, ref_path.parent, _seen=_seen, _depth=_depth + 1) + # Recursively resolve refs in the loaded content — relative to the + # fragment's own directory, but confined to the ORIGINAL root. + result = _resolve_refs( + resolved, + ref_path.parent, + ref_root=root, + _source=ref_path, + _loc=_pointer_parts(pointer), + _seen=_seen, + _depth=_depth + 1, + ) # Pop from ancestry stack so sibling branches can ref the same file _seen.discard(ref_key) @@ -362,21 +416,78 @@ def _resolve_refs( # ── Recurse into dicts ──────────────────────────────────────── if isinstance(obj, dict): - return {k: _resolve_refs(v, base_dir, _seen=_seen, _depth=_depth) for k, v in obj.items()} + return { + k: _resolve_refs( + v, + base_dir, + ref_root=root, + _source=_source, + _loc=(*_loc, k), + _seen=_seen, + _depth=_depth, + ) + for k, v in obj.items() + } # ── Recurse into lists ──────────────────────────────────────── if isinstance(obj, list): - return [_resolve_refs(item, base_dir, _seen=_seen, _depth=_depth) for item in obj] + return [ + _resolve_refs( + item, + base_dir, + ref_root=root, + _source=_source, + _loc=(*_loc, i), + _seen=_seen, + _depth=_depth, + ) + for i, item in enumerate(obj) + ] # ── Scalars pass through ────────────────────────────────────── return obj +def _effective_ref_root( + contract_path: Path, + contract: Any, + ref_root: Optional[Union[str, Path]], +) -> Path: + """The directory every ``$ref`` of *contract* must stay inside. + + Default: the directory of the root contract file (symlinks resolved). + Widened only by an explicit caller choice — the ``ref_root`` argument, + else the ``FLUID_REF_ROOT`` environment variable — and even then the + contract itself must live inside the wider root. A blank variable counts + as unset (the confined default), never as "no confinement". + + The opt-in is only consulted when the contract has an external ref, so a + stale ``FLUID_REF_ROOT`` in a shell cannot break ref-free contracts. + """ + contract_dir = contract_path.resolve().parent + if ref_root is not None: + explicit, origin = str(ref_root), "ref_root" + else: + explicit, origin = os.environ.get(REF_ROOT_ENV, "").strip(), REF_ROOT_ENV + if not explicit or next(iter_external_refs(contract), None) is None: + return contract_dir + root = Path(explicit).expanduser().resolve() + if not root.is_dir(): + raise RefResolutionError(f"{origin}={explicit!r} is not a directory (resolved to {root})") + if not contract_dir.is_relative_to(root): + raise RefResolutionError( + f"contract {contract_path} is outside {origin}={explicit!r} " + f"(resolved to {root}); the ref root must contain the contract" + ) + return root + + def compile_contract( path: Union[str, Path], *, resolve_refs: bool = True, logger: Optional[logging.Logger] = None, + ref_root: Optional[Union[str, Path]] = None, ) -> Dict[str, Any]: """Load a contract and resolve all ``$ref`` pointers into a single document. @@ -387,6 +498,9 @@ def compile_contract( path: Path to the root contract file. resolve_refs: If False, skip ref resolution (for debugging). logger: Optional logger for diagnostics. + ref_root: Directory every ``$ref`` must stay inside. Default: the + root contract's directory (or ``FLUID_REF_ROOT`` when set). Must + contain the contract. See ``docs/contract-refs.md``. Returns: Fully resolved contract dict with no remaining ``$ref`` nodes @@ -400,7 +514,8 @@ def compile_contract( return contract log.info("compile_start", extra={"path": str(p)}) - compiled = _resolve_refs(contract, p.parent) + root = _effective_ref_root(p, contract, ref_root) + compiled = _resolve_refs(contract, p.parent, ref_root=root, _source=p) log.info("compile_done", extra={"path": str(p)}) return compiled @@ -665,18 +780,30 @@ def note_missing_overlay( ) -def load_contract(path: str | Path, *, resolve_refs: bool = True) -> Dict[str, Any]: +def load_contract( + path: str | Path, + *, + resolve_refs: bool = True, + ref_root: Optional[Union[str, Path]] = None, +) -> Dict[str, Any]: """ Load a single FLUID contract file (JSON or YAML). By default, any ``$ref`` pointers are resolved transparently so callers always receive a fully-expanded document. Pass ``resolve_refs=False`` to load the raw document without expansion. + + ``$ref`` targets are confined to the contract's directory tree; pass + ``ref_root`` (or set ``FLUID_REF_ROOT``) to widen it to a directory that + contains the contract. A ref outside the root raises + :class:`RefConfinementError`. """ p = Path(path) contract = _parse_file(p) if resolve_refs: - contract = _resolve_refs(contract, p.resolve().parent) + source = p.resolve() + root = _effective_ref_root(p, contract, ref_root) + contract = _resolve_refs(contract, source.parent, ref_root=root, _source=source) return contract @@ -713,6 +840,7 @@ def load_with_overlay( logger: Optional[logging.Logger] = None, *, resolve_refs: bool = True, + ref_root: Optional[Union[str, Path]] = None, ) -> Dict[str, Any]: """ Load a contract and, if env is provided, deep-merge a matching overlay. @@ -730,7 +858,7 @@ def load_with_overlay( base_path = Path(contract_path) # Load base (with ref resolution) - base = load_contract(base_path, resolve_refs=resolve_refs) + base = load_contract(base_path, resolve_refs=resolve_refs, ref_root=ref_root) # Apply overlay if requested if env: diff --git a/fluid_build/util/ref_confinement.py b/fluid_build/util/ref_confinement.py new file mode 100644 index 00000000..7f3c796a --- /dev/null +++ b/fluid_build/util/ref_confinement.py @@ -0,0 +1,221 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""The one confinement check every external ``$ref`` goes through. + +A contract is untrusted input: the Command Center runs ``fluid validate`` / +``plan`` / ``bundle`` on contracts users upload. Composing a file into the +contract and then echoing the contract back (``bundle`` prints it, ``validate`` +quotes it in errors) turns an unconfined ``$ref`` into a read of any +dict-rooted YAML/JSON file on the host. So a ``$ref`` may only name a file +inside the *ref root*: by default the directory of the root contract file. + +The rule, in the order it is applied: + +1. A ref with a URL scheme (``file://``, ``http://``, ``s3://``, ...) or a + scheme-relative ``//host/...`` form is treated as **remote** and refused. + Nothing in FLUID fetches remote refs. +2. An absolute path (POSIX ``/x``, Windows ``C:\\x`` / ``\\\\server\\x``) is + refused. +3. The ref is joined to the directory of the file that contains it and + ``Path.resolve()``-d, which collapses ``..`` AND follows symlinks; the + result must satisfy ``is_relative_to(root)`` with ``root`` resolved the + same way. A ``../`` climb or a symlink pointing out of the root fails here. +4. With no root at all (a fragment that has no directory, e.g. an OpenAPI + document extracted from a bundle), every external ref is refused: only + same-document ``#/...`` refs remain. + +Borrowed, not built — the shape is the one two maintained OSS resolvers +converged on: + +* datamodel-code-generator >= 0.62.0, the containment fix for + CVE-2026-55389: resolve the candidate, then require + ``is_relative_to(base_path)``; ``file://`` is treated as a remote ref; + widening the base is an explicit caller opt-in. +* python-jsonschema/referencing's documented filesystem retriever: check the + URI against an allowed prefix before reading and raise ``NoSuchResource`` + otherwise. + +Neither is imported: FLUID's resolver composes YAML fragments with a +JSON-pointer suffix, which neither library's resolver does, so the policy is +borrowed and the resolver stays ``fluid_build.loader._resolve_refs``. + +Stdlib only, so both ``fluid_build.loader`` and +``fluid_build.forge.core.validators`` can import it without new edges. +""" + +from __future__ import annotations + +import re +from pathlib import Path, PurePosixPath, PureWindowsPath +from typing import Any, Iterator, Optional, Sequence, Tuple, Union + +__all__ = [ + "REF_ROOT_ENV", + "RefConfinementError", + "RefResolutionError", + "confine_ref", + "format_pointer", + "iter_external_refs", +] + +#: Environment variable a CLI caller sets to widen the ref root (e.g. to a +#: monorepo root holding shared fragments). Read by ``fluid_build.loader``. +REF_ROOT_ENV = "FLUID_REF_ROOT" + +# RFC 3986 scheme: ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ) ":". +_SCHEME_RE = re.compile(r"^[A-Za-z][A-Za-z0-9+.\-]*:") +# A Windows drive ("C:" / "C:\x" / "C:x") also matches the scheme regex; it is +# a path, and is refused as absolute (drive-relative ``C:x`` included) instead. +_DRIVE_RE = re.compile(r"^[A-Za-z]:") + + +class RefResolutionError(Exception): + """Raised when a ``$ref`` cannot be resolved.""" + + +class RefConfinementError(RefResolutionError): + """A ``$ref`` names something outside the ref root. + + Subclasses :class:`RefResolutionError`, so every existing + ``except RefResolutionError`` (``fluid bundle``, the contract loader's + ``contract_load_failed`` path) handles it unchanged. The attributes let a + caller such as the Command Center report the offending ref without + parsing the message. + """ + + def __init__( + self, + message: str, + *, + ref: str, + pointer: str, + source: Optional[str] = None, + root: Optional[str] = None, + ) -> None: + super().__init__(message) + self.ref = ref + self.pointer = pointer + self.source = source + self.root = root + + +def format_pointer(parts: Sequence[Union[str, int]]) -> str: + """Render path segments as an RFC 6901 JSON pointer (``""`` is the root).""" + return "".join("/" + str(p).replace("~", "~0").replace("/", "~1") for p in parts) + + +def _where(pointer: str, source: Optional[Path]) -> str: + at = f"at JSON pointer '{pointer}'" if pointer else "at the document root" + return f"{at} in {source}" if source is not None else at + + +def _is_absolute(file_part: str) -> bool: + return ( + PurePosixPath(file_part).is_absolute() + or PureWindowsPath(file_part).is_absolute() + or bool(_DRIVE_RE.match(file_part)) + or file_part.startswith("\\") + ) + + +def confine_ref( + ref: str, + file_part: str, + *, + base_dir: Optional[Path], + root: Optional[Path], + pointer: str = "", + source: Optional[Path] = None, + root_hint: str = "", +) -> Path: + """Return the resolved target of an external ``$ref``, or refuse it. + + Args: + ref: The ``$ref`` value exactly as written (used in messages). + file_part: The part of *ref* before ``#`` (non-empty). + base_dir: Directory of the file that contains the ref; relative + refs are joined to it. + root: The ref root every target must stay inside. ``None`` means the + document has no filesystem home, so every external ref is refused. + pointer: JSON pointer of the ``$ref`` node inside *source*. + source: The file containing the ref, for the message. + root_hint: Appended to the escape message (how to widen the root). + + Raises: + RefConfinementError: the ref is a URL, an absolute path, escapes + *root* (after symlinks are resolved), or *root* is ``None``. + """ + where = _where(pointer, source) + + def _refuse(reason: str) -> RefConfinementError: + return RefConfinementError( + f"$ref '{ref}' {where} {reason}", + ref=ref, + pointer=pointer, + source=str(source) if source is not None else None, + root=str(root) if root is not None else None, + ) + + if file_part.startswith("//") or ( + _SCHEME_RE.match(file_part) and not _DRIVE_RE.match(file_part) + ): + raise _refuse( + "is a URL; remote refs (including file://) are not supported" + + (" — use a relative path to a file inside the ref root" if root else "") + ) + if _is_absolute(file_part): + raise _refuse( + "must be a relative path, got absolute" + + (" — use a path relative to the file that contains the ref" if root else "") + ) + if root is None or base_dir is None: + raise _refuse( + "points at another file, but this document has no base directory; " + "only same-document refs ('#/...') are allowed here" + ) + + try: + resolved_root = root.resolve() + target = (base_dir / file_part).resolve() + except (OSError, ValueError, RuntimeError) as exc: # NUL byte, symlink loop + raise _refuse(f"cannot be resolved as a path: {exc}") from exc + + if not target.is_relative_to(resolved_root): + hint = f" {root_hint}" if root_hint else "" + raise _refuse( + f"escapes the ref root {resolved_root} (after resolving '..' and " + f"symlinks); refs may only name files inside it.{hint}" + ) + return target + + +def iter_external_refs(doc: Any) -> Iterator[Tuple[str, str]]: + """Yield ``(json_pointer, ref)`` for every ``$ref`` that is not ``#...``. + + Walks dicts and lists iteratively (no recursion limit to hit on a deep + document). Non-string ``$ref`` values are skipped: they resolve nothing. + """ + stack: list[Tuple[Tuple[Union[str, int], ...], Any]] = [((), doc)] + while stack: + parts, node = stack.pop() + if isinstance(node, dict): + ref = node.get("$ref") + if isinstance(ref, str) and not ref.startswith("#"): + yield format_pointer(parts), ref + for key, value in node.items(): + stack.append(((*parts, key), value)) + elif isinstance(node, list): + for idx, value in enumerate(node): + stack.append(((*parts, idx), value)) diff --git a/tests/forge/test_openapi_external_refs.py b/tests/forge/test_openapi_external_refs.py new file mode 100644 index 00000000..1ed07784 --- /dev/null +++ b/tests/forge/test_openapi_external_refs.py @@ -0,0 +1,110 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Bundled OpenAPI fragments may not ``$ref`` outside themselves. + +``fluid validate `` hands each ``sources/openapi/*`` fragment to +openapi-spec-validator, whose default handlers follow ``file://`` (reading a +host file, whose values then appear in the validation error) and +``http(s)://`` (an outbound request). A bundled fragment has no directory of +its own, so only same-document ``#/...`` refs are allowed and the validator +is never called on a fragment with any other ref. +""" + +from __future__ import annotations + +import json +import sys +import types +from unittest.mock import patch + +import pytest + +from fluid_build.forge.core.validators import validate_openapi + + +def _spec(ref: str) -> bytes: + spec = { + "openapi": "3.0.3", + "info": {"title": "t", "version": "1"}, + "paths": { + "/a": { + "get": { + "responses": { + "200": { + "description": "ok", + "content": {"application/json": {"schema": {"$ref": ref}}}, + } + } + } + } + }, + "components": {"schemas": {"A": {"type": "object"}}}, + } + return json.dumps(spec).encode() + + +_POINTER = "/paths/~1a/get/responses/200/content/application~1json/schema" + + +@pytest.fixture +def fake_validator(monkeypatch): + """A stand-in openapi_spec_validator that records every call.""" + calls = [] + mod = types.ModuleType("openapi_spec_validator") + mod.validate = lambda spec: calls.append(spec) # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "openapi_spec_validator", mod) + with patch( + "fluid_build.forge.core.validators._openapi_validator_available", + return_value=True, + ): + yield calls + + +@pytest.mark.parametrize( + "ref, reason", + [ + ("file:///etc/hosts", "is a URL"), + ("http://169.254.169.254/latest/meta-data", "is a URL"), + ("https://example.com/schemas.yaml#/A", "is a URL"), + ("/etc/hosts", "must be a relative path"), + ("../outside.yaml", "no base directory"), + ("other.yaml#/A", "no base directory"), + ], +) +def test_external_ref_is_reported_and_validator_not_called(fake_validator, ref, reason): + issues = validate_openapi("sources/openapi/x.json", _spec(ref), strict=False) + assert fake_validator == [] + assert [i.code for i in issues] == ["OAS-REF-EXTERNAL"] + assert issues[0].severity == "error" + assert f"'{ref}'" in issues[0].message + assert f"JSON pointer '{_POINTER}'" in issues[0].message + assert reason in issues[0].message + + +def test_same_document_ref_still_validated(fake_validator): + issues = validate_openapi("s.json", _spec("#/components/schemas/A"), strict=False) + assert issues == [] + assert len(fake_validator) == 1 + + +def test_real_validator_never_reads_the_file(tmp_path): + """Against the real library: a ``file://`` ref to a file whose content is + an invalid schema would surface that content in the error if followed.""" + pytest.importorskip("openapi_spec_validator") + target = tmp_path / "bad.json" + target.write_text(json.dumps({"type": 12345}), encoding="utf-8") + issues = validate_openapi("s.json", _spec(target.as_uri()), strict=False) + assert [i.code for i in issues] == ["OAS-REF-EXTERNAL"] + assert "12345" not in issues[0].message diff --git a/tests/test_loader_ref_confinement.py b/tests/test_loader_ref_confinement.py new file mode 100644 index 00000000..675ab095 --- /dev/null +++ b/tests/test_loader_ref_confinement.py @@ -0,0 +1,387 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""``$ref`` targets are confined to the root contract's directory tree. + +Before this, a relative ``$ref`` was joined to the contract's directory with +``../`` honoured, and only absolute paths and system directories were blocked, +so a contract could compose any dict-rooted YAML/JSON file on the host into +itself — and ``fluid bundle`` printed it back. Every test below builds the +layout on disk and goes through the public loader entry points (or the real +``fluid`` CLI), with a secret file sitting just outside the contract directory. +""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +import pytest +import yaml + +from fluid_build.loader import ( + REF_ROOT_ENV, + RefConfinementError, + RefResolutionError, + compile_contract, + load_contract, + load_with_overlay, +) + +SECRET = "SUPER-SECRET-VALUE" + + +def _write(path: Path, data) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(yaml.safe_dump(data, sort_keys=False), encoding="utf-8") + return path + + +@pytest.fixture +def layout(tmp_path, monkeypatch): + """``tmp/outside/secret.yaml`` next to ``tmp/proj/`` (the contract dir).""" + monkeypatch.delenv(REF_ROOT_ENV, raising=False) + _write(tmp_path / "outside" / "secret.yaml", {"api_key": SECRET}) + proj = tmp_path / "proj" + proj.mkdir() + return tmp_path, proj + + +def _contract(proj: Path, ref: str, name: str = "contract.fluid.yaml") -> Path: + return _write(proj / name, {"id": "p", "labels": {"$ref": ref}}) + + +def _symlink(link: Path, target: Path, *, is_dir: bool = False) -> None: + try: + link.symlink_to(target, target_is_directory=is_dir) + except (OSError, NotImplementedError): # Windows without symlink privilege + pytest.skip("symlinks unavailable on this platform") + + +# --------------------------------------------------------------------------- +# Escapes are refused — by every entry point +# --------------------------------------------------------------------------- + + +class TestEscapesRefused: + @pytest.mark.parametrize("entry", ["load_contract", "compile_contract", "load_with_overlay"]) + def test_dotdot_escape_refused_with_ref_and_pointer(self, layout, entry): + _, proj = layout + contract = _contract(proj, "../outside/secret.yaml") + fn = { + "load_contract": load_contract, + "compile_contract": compile_contract, + "load_with_overlay": load_with_overlay, + }[entry] + with pytest.raises(RefConfinementError) as excinfo: + fn(contract) + err = excinfo.value + msg = str(err) + assert "'../outside/secret.yaml'" in msg + assert "JSON pointer '/labels'" in msg + assert "escapes the ref root" in msg + assert SECRET not in msg + assert err.ref == "../outside/secret.yaml" + assert err.pointer == "/labels" + assert err.source == str(contract.resolve()) + # Still the loader's typed error, so existing handlers catch it. + assert isinstance(err, RefResolutionError) + + def test_absolute_path_refused_even_inside_root(self, layout): + _, proj = layout + inside = _write(proj / "frag.yaml", {"a": 1}) + contract = _contract(proj, str(inside.resolve())) + with pytest.raises(RefConfinementError, match="must be a relative path"): + load_contract(contract) + + @pytest.mark.parametrize( + "ref", + [ + "C:\\Windows\\win.ini", + "C:relative-to-drive.yaml", + "\\\\server\\share\\x.yaml", + "\\rooted.yaml", + ], + ) + def test_windows_shaped_absolute_paths_refused(self, layout, ref): + _, proj = layout + with pytest.raises(RefConfinementError, match="must be a relative path"): + load_contract(_contract(proj, ref)) + + def test_file_url_refused_even_when_target_is_inside_root(self, layout): + _, proj = layout + inside = _write(proj / "frag.yaml", {"a": 1}) + contract = _contract(proj, inside.resolve().as_uri()) + with pytest.raises(RefConfinementError, match="is a URL"): + load_contract(contract) + + @pytest.mark.parametrize( + "ref", + [ + "http://169.254.169.254/latest/meta-data/x.yaml", + "https://example.com/frag.yaml#/x", + "s3://bucket/frag.yaml", + "FILE:///etc/hosts", + "//example.com/frag.yaml", + ], + ) + def test_remote_refs_refused(self, layout, ref): + _, proj = layout + with pytest.raises(RefConfinementError, match="is a URL"): + load_contract(_contract(proj, ref)) + + def test_symlinked_file_pointing_out_is_refused(self, layout): + tmp, proj = layout + _symlink(proj / "innocent.yaml", tmp / "outside" / "secret.yaml") + with pytest.raises(RefConfinementError, match="escapes the ref root"): + load_contract(_contract(proj, "./innocent.yaml")) + + def test_symlinked_directory_pointing_out_is_refused(self, layout): + tmp, proj = layout + _symlink(proj / "shared", tmp / "outside", is_dir=True) + with pytest.raises(RefConfinementError, match="escapes the ref root"): + load_contract(_contract(proj, "./shared/secret.yaml")) + + def test_nested_ref_from_subdirectory_is_held_to_the_root(self, layout): + """``sub/frag.yaml`` refs ``../../outside/...``: relative to the + fragment that is only one level out of the root — still refused, and + the error names the fragment and the pointer inside it.""" + _, proj = layout + frag = _write( + proj / "sub" / "frag.yaml", + {"inner": [{"$ref": "../../outside/secret.yaml"}]}, + ) + contract = _contract(proj, "./sub/frag.yaml") + with pytest.raises(RefConfinementError) as excinfo: + load_contract(contract) + assert excinfo.value.source == str(frag.resolve()) + assert excinfo.value.pointer == "/inner/0" + + def test_nested_ref_location_follows_the_fragment_pointer(self, layout): + _, proj = layout + _write( + proj / "frag.yaml", + {"section": {"deep": {"$ref": "../outside/secret.yaml"}}}, + ) + contract = _contract(proj, "./frag.yaml#/section") + with pytest.raises(RefConfinementError) as excinfo: + load_contract(contract) + assert excinfo.value.pointer == "/section/deep" + + def test_refusal_does_not_reveal_whether_the_target_exists(self, layout): + """Confinement runs before the existence check: an escaping ref to a + file that does not exist fails the same way as one that does.""" + _, proj = layout + with pytest.raises(RefConfinementError, match="escapes the ref root"): + load_contract(_contract(proj, "../outside/no-such-file.yaml")) + + +# --------------------------------------------------------------------------- +# Legitimate composition keeps working +# --------------------------------------------------------------------------- + + +class TestLegitimateRefsStillResolve: + def test_sibling_and_subdirectory_refs(self, layout): + _, proj = layout + _write(proj / "owner.yaml", {"team": "data"}) + _write(proj / "fragments" / "builds" / "ingest.yaml", {"id": "ingest"}) + contract = _write( + proj / "contract.fluid.yaml", + { + "owner": {"$ref": "./owner.yaml"}, + "builds": [{"$ref": "fragments/builds/ingest.yaml"}], + }, + ) + result = load_contract(contract) + assert result == {"owner": {"team": "data"}, "builds": [{"id": "ingest"}]} + + def test_nested_ref_climbing_back_inside_root(self, layout): + """A fragment in ``sub/`` may ref ``../common.yaml``: relative to the + fragment, inside the root — allowed.""" + _, proj = layout + _write(proj / "common.yaml", {"tier": "gold"}) + _write(proj / "sub" / "frag.yaml", {"common": {"$ref": "../common.yaml"}}) + result = load_contract(_contract(proj, "./sub/frag.yaml")) + assert result["labels"] == {"common": {"tier": "gold"}} + + def test_symlink_that_stays_inside_root(self, layout): + _, proj = layout + real = _write(proj / "real" / "frag.yaml", {"ok": True}) + _symlink(proj / "alias.yaml", real) + assert load_contract(_contract(proj, "./alias.yaml"))["labels"] == {"ok": True} + + def test_contract_reached_through_a_symlinked_directory(self, tmp_path, monkeypatch): + """The root is resolved too, so a contract opened via a symlinked + path (``/tmp`` → ``/private/tmp`` on macOS) is not refused.""" + monkeypatch.delenv(REF_ROOT_ENV, raising=False) + real = tmp_path / "real" + _write(real / "frag.yaml", {"ok": True}) + _contract(real, "./frag.yaml") + _symlink(tmp_path / "link", real, is_dir=True) + result = load_contract(tmp_path / "link" / "contract.fluid.yaml") + assert result["labels"] == {"ok": True} + + def test_same_document_refs_are_left_in_place(self, layout): + _, proj = layout + contract = _write( + proj / "contract.fluid.yaml", + {"defs": {"x": 1}, "use": {"$ref": "#/defs/x"}, "whole": {"$ref": "#"}}, + ) + result = load_contract(contract) + assert result["use"] == {"$ref": "#/defs/x"} + assert result["whole"] == {"$ref": "#"} + + +# --------------------------------------------------------------------------- +# The explicit opt-in: a caller-set ref root +# --------------------------------------------------------------------------- + + +class TestRefRootOptIn: + def _monorepo(self, tmp: Path) -> Path: + _write(tmp / "shared" / "policy.yaml", {"classification": "Internal"}) + product = tmp / "products" / "orders" + return _contract(product, "../../shared/policy.yaml") + + def test_ref_root_argument_widens_the_root(self, tmp_path, monkeypatch): + monkeypatch.delenv(REF_ROOT_ENV, raising=False) + contract = self._monorepo(tmp_path) + with pytest.raises(RefConfinementError): + load_contract(contract) + result = load_contract(contract, ref_root=tmp_path) + assert result["labels"] == {"classification": "Internal"} + assert compile_contract(contract, ref_root=tmp_path)["labels"] == result["labels"] + assert load_with_overlay(contract, ref_root=tmp_path)["labels"] == result["labels"] + + def test_env_var_widens_the_root(self, tmp_path, monkeypatch): + contract = self._monorepo(tmp_path) + monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path)) + assert load_contract(contract)["labels"] == {"classification": "Internal"} + + def test_widened_root_still_confines(self, tmp_path, monkeypatch): + _write(tmp_path / "secret.yaml", {"api_key": SECRET}) + repo = tmp_path / "repo" + contract = _contract(repo / "products" / "orders", "../../../secret.yaml") + monkeypatch.setenv(REF_ROOT_ENV, str(repo)) + with pytest.raises(RefConfinementError, match="escapes the ref root"): + load_contract(contract) + + def test_blank_env_var_means_the_confined_default(self, tmp_path, monkeypatch): + contract = self._monorepo(tmp_path) + monkeypatch.setenv(REF_ROOT_ENV, " ") + with pytest.raises(RefConfinementError): + load_contract(contract) + + def test_root_that_does_not_contain_the_contract_is_rejected(self, tmp_path, monkeypatch): + contract = self._monorepo(tmp_path) + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + monkeypatch.setenv(REF_ROOT_ENV, str(elsewhere)) + with pytest.raises(RefResolutionError, match="the ref root must contain the contract"): + load_contract(contract) + + def test_root_that_is_not_a_directory_is_rejected(self, tmp_path, monkeypatch): + contract = self._monorepo(tmp_path) + with pytest.raises(RefResolutionError, match="is not a directory"): + load_contract(contract, ref_root=tmp_path / "missing") + monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path / "missing")) + with pytest.raises(RefResolutionError, match=f"{REF_ROOT_ENV}=.* is not a directory"): + load_contract(contract) + + def test_stale_env_var_does_not_break_ref_free_contracts(self, tmp_path, monkeypatch): + monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path / "missing")) + contract = _write(tmp_path / "c.yaml", {"id": "p", "same": {"$ref": "#/id"}}) + assert load_contract(contract)["id"] == "p" + + def test_url_refs_stay_refused_under_the_opt_in(self, tmp_path, monkeypatch): + contract = _contract(tmp_path / "p", "file:///etc/hosts") + monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path)) + with pytest.raises(RefConfinementError, match="is a URL"): + load_contract(contract) + + +# --------------------------------------------------------------------------- +# The real CLI +# --------------------------------------------------------------------------- + + +def _fluid(*args: str, cwd: Path, env_extra: dict | None = None) -> subprocess.CompletedProcess: + env = os.environ.copy() + env.pop(REF_ROOT_ENV, None) + env.update(env_extra or {}) + return subprocess.run( + [sys.executable, "-m", "fluid_build.cli", *args], + cwd=cwd, + env=env, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=120, + ) + + +_VALID_CONTRACT = { + "fluidVersion": "0.7.5", + "kind": "DataProduct", + "id": "example.ref_confinement", + "name": "Ref confinement", + "domain": "example", + "metadata": {"layer": "Bronze", "owner": {"team": "t", "email": "t@example.com"}}, + "labels": {"$ref": "../outside/secret.yaml"}, + "exposes": [ + { + "exposeId": "out", + "kind": "table", + "binding": { + "platform": "local", + "format": "csv", + "location": {"path": "runtime/out/x.csv"}, + }, + "contract": {"schema": [{"name": "message", "type": "string"}]}, + } + ], +} + + +@pytest.mark.integration +class TestCli: + def test_validate_refuses_an_escaping_contract(self, layout): + _, proj = layout + contract = _write(proj / "contract.fluid.yaml", _VALID_CONTRACT) + result = _fluid("validate", str(contract), cwd=proj) + # The Rich error panel wraps long lines; compare whitespace-normalised. + out = " ".join((result.stdout + result.stderr).split()) + assert result.returncode != 0, out + assert "../outside/secret.yaml" in out + assert "escapes the ref root" in out + assert SECRET not in out + + def test_bundle_does_not_print_the_outside_file(self, layout): + _, proj = layout + contract = _write(proj / "contract.fluid.yaml", _VALID_CONTRACT) + result = _fluid("bundle", str(contract), cwd=proj) + out = result.stdout + result.stderr + assert result.returncode == 2, out + assert "escapes the ref root" in out + assert SECRET not in out + + def test_validate_accepts_it_under_the_documented_opt_in(self, layout): + tmp, proj = layout + contract = _write(proj / "contract.fluid.yaml", _VALID_CONTRACT) + result = _fluid("validate", str(contract), cwd=proj, env_extra={REF_ROOT_ENV: str(tmp)}) + assert result.returncode == 0, result.stdout + result.stderr diff --git a/tests/test_loader_refs.py b/tests/test_loader_refs.py index 85b28ddd..635fd956 100644 --- a/tests/test_loader_refs.py +++ b/tests/test_loader_refs.py @@ -334,7 +334,9 @@ class TestResolveRefsPathVariants: """Path variation edge cases.""" def test_parent_directory_traversal(self, tmp_path): - """Ref with ../ to a sibling directory.""" + """Ref with ../ to a sibling directory resolves only once the caller + widens the ref root to a directory holding both; by default the + contract's own directory is the root and the ref is refused.""" dir_a = tmp_path / "a" dir_b = tmp_path / "b" dir_a.mkdir() @@ -343,7 +345,9 @@ def test_parent_directory_traversal(self, tmp_path): _write_yaml(dir_b / "shared.yaml", {"data": "from_sibling"}) _write_yaml(dir_a / "contract.yaml", {"section": {"$ref": "../b/shared.yaml"}}) - result = load_contract(dir_a / "contract.yaml") + with pytest.raises(RefResolutionError, match="escapes the ref root"): + load_contract(dir_a / "contract.yaml") + result = load_contract(dir_a / "contract.yaml", ref_root=tmp_path) assert result["section"]["data"] == "from_sibling" def test_yml_extension(self, tmp_path): @@ -921,7 +925,11 @@ def test_no_overlay_returns_base_unchanged(self, tmp_path): class TestRefResolverPlatformAwareBlocking: """The ``$ref`` resolver must reject refs that traverse into a system directory, using the same platform-aware deny set as the - rest of the CLI (``cli/security.py``).""" + rest of the CLI (``cli/security.py``). + + Confinement to the ref root now refuses these first; the traversal + tests widen the root to the filesystem anchor so they still prove + the deny list is a working second layer for a caller that opts in.""" def test_absolute_ref_still_rejected(self, tmp_path): """An absolute ``$ref`` path is rejected outright (relative-only @@ -939,8 +947,11 @@ def test_traversal_ref_into_etc_rejected(self, tmp_path): climb = "/".join([".."] * depth) ref = f"./{climb}/etc/passwd" if climb else "./etc/passwd" tree = {"section": {"$ref": ref}} - with pytest.raises(RefResolutionError, match="blocked system path"): + with pytest.raises(RefResolutionError, match="escapes the ref root"): _resolve_refs(tree, tmp_path) + anchor = Path(tmp_path.resolve().anchor) + with pytest.raises(RefResolutionError, match="blocked system path"): + _resolve_refs(tree, tmp_path, ref_root=anchor) @pytest.mark.skipif( not sys.platform.startswith("darwin"), @@ -959,15 +970,15 @@ def test_traversal_ref_into_private_etc_rejected_on_macos(self, tmp_path): ref = f"./{climb}/private/etc/passwd" if climb else "./private/etc/passwd" tree = {"section": {"$ref": ref}} with pytest.raises(RefResolutionError, match="blocked system path"): - _resolve_refs(tree, tmp_path) + _resolve_refs(tree, tmp_path, ref_root=Path("/")) def test_legitimate_sibling_ref_still_allowed(self, tmp_path): - """F3 must not over-block: a normal relative ``../sibling/`` - ref inside the project tree still resolves (monorepo layouts).""" + """F3 must not over-block: a ``../sibling/`` ref inside a widened + ref root (the monorepo opt-in) still resolves.""" sibling = tmp_path / "shared" _write_yaml(sibling / "common.yaml", {"classification": "Internal"}) product = tmp_path / "product" product.mkdir() tree = {"policy": {"$ref": "../shared/common.yaml"}} - result = _resolve_refs(tree, product) + result = _resolve_refs(tree, product, ref_root=tmp_path) assert result["policy"]["classification"] == "Internal" diff --git a/tests/util/test_ref_confinement.py b/tests/util/test_ref_confinement.py new file mode 100644 index 00000000..39315abd --- /dev/null +++ b/tests/util/test_ref_confinement.py @@ -0,0 +1,81 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unit tests for the shared ``$ref`` confinement primitive.""" + +from __future__ import annotations + +import pytest + +from fluid_build.util.ref_confinement import ( + RefConfinementError, + confine_ref, + format_pointer, + iter_external_refs, +) + + +@pytest.mark.parametrize( + "parts, expected", + [ + ((), ""), + (("a", 0, "b"), "/a/0/b"), + (("a/b", "m~n"), "/a~1b/m~0n"), + ], +) +def test_format_pointer_is_rfc6901(parts, expected): + assert format_pointer(parts) == expected + + +def test_iter_external_refs_skips_same_document_and_non_string_refs(): + doc = { + "a": {"$ref": "#/x"}, + "b": [{"$ref": "./f.yaml"}, {"$ref": 3}], + "c": {"d": {"$ref": "http://h/x", "extra": 1}}, + } + assert sorted(iter_external_refs(doc)) == [("/b/0", "./f.yaml"), ("/c/d", "http://h/x")] + + +def test_inside_root_returns_resolved_target(tmp_path): + (tmp_path / "sub").mkdir() + target = confine_ref("sub/x.yaml", "sub/x.yaml", base_dir=tmp_path, root=tmp_path) + assert target == (tmp_path / "sub" / "x.yaml").resolve() + + +def test_root_itself_is_inside(tmp_path): + sub = tmp_path / "sub" + sub.mkdir() + assert confine_ref("..", "..", base_dir=sub, root=tmp_path) == tmp_path.resolve() + + +def test_prefix_sibling_is_not_inside(tmp_path): + """``/x/proj-evil`` shares a string prefix with ``/x/proj`` but is not + inside it — the check is path-wise, not ``str.startswith``.""" + proj = tmp_path / "proj" + proj.mkdir() + with pytest.raises(RefConfinementError, match="escapes the ref root"): + confine_ref("../proj-evil/x.yaml", "../proj-evil/x.yaml", base_dir=proj, root=proj) + + +def test_nul_byte_is_refused_as_typed_error(tmp_path): + with pytest.raises(RefConfinementError): + confine_ref("a\0b", "a\0b", base_dir=tmp_path, root=tmp_path) + + +def test_root_hint_is_appended_only_to_escapes(tmp_path): + with pytest.raises(RefConfinementError, match="HINT"): + confine_ref("../x", "../x", base_dir=tmp_path, root=tmp_path, root_hint="HINT") + with pytest.raises(RefConfinementError) as excinfo: + confine_ref("http://h/x", "http://h/x", base_dir=tmp_path, root=tmp_path, root_hint="HINT") + assert "HINT" not in str(excinfo.value) From 4b0ee8673d9b8feed124bfe34817e9ea64aa0cda Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 14:42:58 +0200 Subject: [PATCH 2/9] fix(loader): a FLUID_REF_ROOT outside the contract falls back, not fails FLUID_REF_ROOT is process-wide. Set once in a shell or a service container, it applies to every contract the process loads. When a contract with a file $ref was outside it (or the value was not a directory), the load raised "contract ... is outside FLUID_REF_ROOT". So `FLUID_REF_ROOT= fluid validate examples/0.7.1/bitcoin-multifile/contract.fluid.yaml` failed, and a platform that sets it once would fail every uploaded contract with a $ref, because each upload is materialised in a fresh temp directory. docs/contract-refs.md said the variable did not affect other contracts. The one test only covered ref-free contracts. _effective_ref_root now handles the two sources differently: - FLUID_REF_ROOT that is not a directory or does not contain the contract is ignored for that contract. It logs a WARNING (event ref_root_env_ignored, once per contract directory and value) and uses the default root. That is the root the contract gets with the variable unset, so nothing is widened, and refs that leave the contract's directory still fail as escapes. - ref_root= is set by the caller for one contract and stays strict: RefResolutionError, naming ref_root. The default confinement also breaks monorepos that shared fragments with `$ref: ../other-product/...`. The bitcoin-multifile README advertised that reuse. The README row and a new section now give the FLUID_REF_ROOT / ref_root= step, checked against the CLI with a sibling product. docs/contract-refs.md gains an upgrade note. OAS-REF-EXTERNAL keeps rejecting a $ref key anywhere in a bundled OpenAPI fragment, including example and x-* payloads that openapi-spec-validator 0.9.0 does not follow. These keys cannot be skipped by name. `properties: {example: {$ref: ...}}` is a schema called "example", and the validator does follow its $ref (checked with an audit hook on open()). This is now documented, and tests pin both the data and the schema positions. --- AGENTS.md | 2 +- docs/contract-refs.md | 63 ++++++++++- examples/0.7.1/bitcoin-multifile/README.md | 31 +++++- fluid_build/loader.py | 57 ++++++++-- tests/forge/test_openapi_external_refs.py | 43 ++++++++ tests/test_loader_ref_confinement.py | 118 +++++++++++++++++++-- 6 files changed, 294 insertions(+), 20 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5e395899..2f733ef2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -231,7 +231,7 @@ A 2026-04-16 security review (see `SECURITY_REVIEW.md` if committed, or the PR # - **Encrypted credential store fails loud.** `credentials/encrypted_store.py::_load_store` raises `CredentialError` on `InvalidToken` — never silently returns `{}` (which previously caused destructive overwrite on next write with the wrong key). - **Tool errors are typed, not text.** `dispatch_tool_call` returns `{"error": , "message": "Tool … failed — see server logs"}` on exception; the full `exc` goes to `LOG.warning(..., exc_info=True)` where the redactor can scrub it. Exception text is never round-tripped into the LLM context (prevents path / hostname / env-var leaks). - **Subprocess argv sanitisation.** `cli/auth.py::_sanitize_argv` redacts values of `--password`, `--token`, `--api-key`, `--key-file`, any flag ending in `-secret`/`-key`/`-token`/`-password`/`-passphrase`. Called from `AuthProvider._run_command` before DEBUG-logging the command. -- **`$ref` resolution is confined.** Every external `$ref` goes through `util/ref_confinement.py::confine_ref`: URL schemes (`file://` included) and absolute paths are refused, and the `Path.resolve()`-d target must be `is_relative_to` the ref root — the root contract's directory unless the caller passes `ref_root=` / sets `FLUID_REF_ROOT`. Nested refs are held to the same root. Bundled OpenAPI fragments have no root, so only `#/...` refs reach openapi-spec-validator. A new `$ref` resolver must call `confine_ref`, not re-implement it. +- **`$ref` resolution is confined.** Every external `$ref` goes through `util/ref_confinement.py::confine_ref`: URL schemes (`file://` included) and absolute paths are refused, and the `Path.resolve()`-d target must be `is_relative_to` the ref root — the root contract's directory unless the caller passes `ref_root=` / sets `FLUID_REF_ROOT` (process-wide, so an env root that is not a directory or does not contain the contract is ignored for that contract with a `ref_root_env_ignored` WARNING; an unusable `ref_root=` raises). Nested refs are held to the same root. Bundled OpenAPI fragments have no root, so only `#/...` refs reach openapi-spec-validator. A new `$ref` resolver must call `confine_ref`, not re-implement it. --- diff --git a/docs/contract-refs.md b/docs/contract-refs.md index 7cfc8472..b5521bbb 100644 --- a/docs/contract-refs.md +++ b/docs/contract-refs.md @@ -107,11 +107,32 @@ contract = load_contract("repo/products/orders/contract.fluid.yaml", ref_root="r Rules for the wider root: -- It must be an existing directory that contains the root contract. If it - does not, the command fails and names the setting it came from. +- It widens the root only for a contract inside it, and only if it is an + existing directory. +- `FLUID_REF_ROOT` applies to every contract the process loads, so it is + ignored for a contract outside it (or when it is not a directory). That + contract gets the default root, its own directory, and a warning says so: + + ```bash + FLUID_REF_ROOT=repo fluid validate /tmp/upload-1234/contract.fluid.yaml + ``` + + ```text + ref_root_env_ignored: contract /tmp/upload-1234/contract.fluid.yaml is outside + FLUID_REF_ROOT='repo' (…); the ref root must contain the contract. Ignoring it + for this contract: its $refs are confined to the contract's own directory + /tmp/upload-1234 (the default). + ``` + + Refs that stay in that directory resolve as usual, and refs that leave it + fail as escapes. A `FLUID_REF_ROOT` left in your shell, or set once for a + service that also loads uploaded contracts from temp directories, never + widens or breaks another contract. +- `ref_root=` is set by the caller for one contract, so it is strict: a + `ref_root` that is not a directory or does not contain the contract raises + `RefResolutionError`, naming `ref_root`. - A blank `FLUID_REF_ROOT` counts as unset: the default root applies. -- It is only consulted when the contract has a ref to another file, so a - `FLUID_REF_ROOT` left in your shell does not affect other contracts. +- It is only consulted when the contract has a ref to another file. - It widens the root and nothing else. URLs and absolute paths are still refused, and refs that resolve into system directories (`/etc`, `/proc`, `/private/etc` on macOS, …) are still blocked. @@ -119,6 +140,21 @@ Rules for the wider root: Set it to the narrowest directory that works. Setting it to `/` turns the confinement off. +### Upgrading: `../` refs to another product's fragments + +Before the ref root existed, a relative ref could climb out of the contract's +directory, so monorepos shared fragments with `$ref: ../other-product/…`. +Those refs now fail with `escapes the ref root` until the root is widened. +Set `FLUID_REF_ROOT` (or `ref_root=`) to the repository root, or to the +narrowest directory that holds the products and their shared fragments: + +```bash +export FLUID_REF_ROOT="$(git rev-parse --show-toplevel)" +fluid validate products/orders/contract.fluid.yaml +``` + +Contracts whose refs stay inside their own directory need no change. + --- ## Catching the error in Python @@ -150,3 +186,22 @@ fragment has no directory. Only same-document refs reported as an `OAS-REF-EXTERNAL` error, and openapi-spec-validator is not run on that fragment. If it were run, it would follow `file://` and `http(s)://` refs. Inline the referenced schemas under `components` instead. + +This applies to a `$ref` key anywhere in the fragment, including inside +`example`, `examples.*.value` and `x-*` extension payloads, which the +validator treats as data: + +```yaml +components: + schemas: + Doc: + type: object + example: + $ref: https://json-schema.org/draft/2020-12/schema # OAS-REF-EXTERNAL +``` + +Those keys are not skipped because the same names are also used for schemas. +`properties: {example: {$ref: …}}` declares a property called `example`, and +openapi-spec-validator does follow that `$ref`. If an example payload has to +contain a `$ref`, rename the key in the payload (for example `ref`) or drop +the example. diff --git a/examples/0.7.1/bitcoin-multifile/README.md b/examples/0.7.1/bitcoin-multifile/README.md index 50f0f837..0202bd8b 100644 --- a/examples/0.7.1/bitcoin-multifile/README.md +++ b/examples/0.7.1/bitcoin-multifile/README.md @@ -48,6 +48,35 @@ fluid apply contract.fluid.yaml --yes |---------|-----| | **Team ownership** | Security team owns `access-policy.yaml`, compliance owns `sovereignty.yaml` | | **Independent versioning** | Add a new expose without touching governance configs | -| **Reusable fragments** | `sovereignty.yaml` can be `$ref`'d by every EU data product | +| **Reusable fragments** | `sovereignty.yaml` can be `$ref`'d by every EU data product, once `FLUID_REF_ROOT` names a directory that holds them all ([below](#sharing-a-fragment-with-another-product)) | | **Smaller diffs** | PRs touch only the fragment that changed | | **The engine stays simple** | validate/plan/apply always receive one resolved document | + +## Sharing a fragment with another product + +A `$ref` may only name a file inside the root contract's directory, so this +contract can reach anything under `bitcoin-multifile/` and nothing outside it. +A second product that wants the same `sovereignty.yaml` has to step outside +its own directory, and that is refused by default: + +```yaml +# examples/0.7.1/eu-orders/contract.fluid.yaml +sovereignty: + $ref: ../bitcoin-multifile/fragments/sovereignty.yaml +``` + +```text +❌ Validation error: contract_load_failed + error: $ref '../bitcoin-multifile/fragments/sovereignty.yaml' at JSON pointer + '/sovereignty' in …/eu-orders/contract.fluid.yaml escapes the ref root …/eu-orders … +``` + +Widen the root to a directory that contains both products: + +```bash +FLUID_REF_ROOT=examples/0.7.1 fluid validate examples/0.7.1/eu-orders/contract.fluid.yaml +``` + +From Python, pass `ref_root="examples/0.7.1"` to `load_contract`. See +[Widening the root](../../../docs/contract-refs.md#widening-the-root-monorepos) +for the rules. diff --git a/fluid_build/loader.py b/fluid_build/loader.py index 61a71091..d6a95cf2 100644 --- a/fluid_build/loader.py +++ b/fluid_build/loader.py @@ -461,8 +461,20 @@ def _effective_ref_root( contract itself must live inside the wider root. A blank variable counts as unset (the confined default), never as "no confinement". - The opt-in is only consulted when the contract has an external ref, so a - stale ``FLUID_REF_ROOT`` in a shell cannot break ref-free contracts. + The opt-in is only consulted when the contract has an external ref. + + The two sources fail differently when the root is unusable (not a + directory, or does not contain the contract): + + * ``ref_root=`` is a choice the caller made for THIS contract, so it is a + :class:`RefResolutionError`. + * ``FLUID_REF_ROOT`` is process-wide: set once in a shell or a service + container, it applies to every contract that process loads, most of + which live elsewhere (a platform materialises each uploaded contract + in a fresh temp directory). It is ignored for such a contract, with a + WARNING, and the contract gets the default root: exactly the root it + would get with the variable unset, so the fallback widens nothing and + refs that leave the contract's directory still fail, as escapes. """ contract_dir = contract_path.resolve().parent if ref_root is not None: @@ -473,13 +485,46 @@ def _effective_ref_root( return contract_dir root = Path(explicit).expanduser().resolve() if not root.is_dir(): - raise RefResolutionError(f"{origin}={explicit!r} is not a directory (resolved to {root})") - if not contract_dir.is_relative_to(root): - raise RefResolutionError( + problem = f"{origin}={explicit!r} is not a directory (resolved to {root})" + elif not contract_dir.is_relative_to(root): + problem = ( f"contract {contract_path} is outside {origin}={explicit!r} " f"(resolved to {root}); the ref root must contain the contract" ) - return root + else: + return root + if origin != REF_ROOT_ENV: + raise RefResolutionError(problem) + _note_ref_root_env_ignored(contract_dir, explicit, problem) + return contract_dir + + +#: (contract directory, FLUID_REF_ROOT value) pairs already reported by +#: :func:`_note_ref_root_env_ignored` in this process — one command loads the +#: same contract several times. Tests reset it with ``.clear()``. +_NOTED_REF_ROOT_ENV_IGNORED: Set[Tuple[str, str]] = set() +_NOTED_REF_ROOT_ENV_IGNORED_LOCK = threading.Lock() + + +def _note_ref_root_env_ignored(contract_dir: Path, value: str, problem: str) -> None: + """WARN, once per (contract directory, value), that ``FLUID_REF_ROOT`` + does not apply to this contract and the default root is used.""" + key = (str(contract_dir), value) + with _NOTED_REF_ROOT_ENV_IGNORED_LOCK: + if key in _NOTED_REF_ROOT_ENV_IGNORED: + return + _NOTED_REF_ROOT_ENV_IGNORED.add(key) + LOG.warning( + "ref_root_env_ignored: %s. Ignoring it for this contract: its $refs are " + "confined to the contract's own directory %s (the default).", + problem, + contract_dir, + extra={ + "event": "ref_root_env_ignored", + "ref_root_env": value, + "contract_dir": str(contract_dir), + }, + ) def compile_contract( diff --git a/tests/forge/test_openapi_external_refs.py b/tests/forge/test_openapi_external_refs.py index 1ed07784..50cace6a 100644 --- a/tests/forge/test_openapi_external_refs.py +++ b/tests/forge/test_openapi_external_refs.py @@ -108,3 +108,46 @@ def test_real_validator_never_reads_the_file(tmp_path): issues = validate_openapi("s.json", _spec(target.as_uri()), strict=False) assert [i.code for i in issues] == ["OAS-REF-EXTERNAL"] assert "12345" not in issues[0].message + + +def _component_spec(schema: dict) -> bytes: + spec = { + "openapi": "3.0.3", + "info": {"title": "t", "version": "1"}, + "paths": {}, + "components": {"schemas": {"Doc": schema}}, + } + return json.dumps(spec).encode() + + +_EXTERNAL = "https://json-schema.org/draft/2020-12/schema" + + +@pytest.mark.parametrize( + "schema, pointer", + [ + # Data-valued positions: openapi-spec-validator 0.9.0 does not follow a + # ``$ref`` here. Still rejected, by a documented blanket rule (below). + ({"type": "object", "example": {"$ref": _EXTERNAL}}, "/components/schemas/Doc/example"), + ({"type": "object", "x-meta": {"$ref": _EXTERNAL}}, "/components/schemas/Doc/x-meta"), + # Schema positions that share those names: the validator DOES follow + # these, so skipping ``example`` / ``x-*`` keys by name would let them + # through to it. + ( + {"type": "object", "properties": {"example": {"$ref": _EXTERNAL}}}, + "/components/schemas/Doc/properties/example", + ), + ( + {"type": "object", "properties": {"x-meta": {"$ref": _EXTERNAL}}}, + "/components/schemas/Doc/properties/x-meta", + ), + ], + ids=["example-payload", "x-extension-payload", "property-named-example", "property-named-x"], +) +def test_ref_key_is_rejected_wherever_it_appears(fake_validator, schema, pointer): + """A ``$ref`` key anywhere in the fragment is an external ref + (docs/contract-refs.md, "OpenAPI fragments inside a bundle").""" + issues = validate_openapi("sources/openapi/x.json", _component_spec(schema), strict=False) + assert fake_validator == [] + assert [i.code for i in issues] == ["OAS-REF-EXTERNAL"] + assert f"JSON pointer '{pointer}'" in issues[0].message diff --git a/tests/test_loader_ref_confinement.py b/tests/test_loader_ref_confinement.py index 675ab095..40fa19eb 100644 --- a/tests/test_loader_ref_confinement.py +++ b/tests/test_loader_ref_confinement.py @@ -24,6 +24,7 @@ from __future__ import annotations +import logging import os import subprocess import sys @@ -32,6 +33,7 @@ import pytest import yaml +from fluid_build import loader from fluid_build.loader import ( REF_ROOT_ENV, RefConfinementError, @@ -286,21 +288,29 @@ def test_blank_env_var_means_the_confined_default(self, tmp_path, monkeypatch): with pytest.raises(RefConfinementError): load_contract(contract) - def test_root_that_does_not_contain_the_contract_is_rejected(self, tmp_path, monkeypatch): + def test_ref_root_argument_that_does_not_contain_the_contract_is_rejected( + self, tmp_path, monkeypatch + ): + monkeypatch.delenv(REF_ROOT_ENV, raising=False) contract = self._monorepo(tmp_path) elsewhere = tmp_path / "elsewhere" elsewhere.mkdir() - monkeypatch.setenv(REF_ROOT_ENV, str(elsewhere)) with pytest.raises(RefResolutionError, match="the ref root must contain the contract"): - load_contract(contract) + load_contract(contract, ref_root=elsewhere) - def test_root_that_is_not_a_directory_is_rejected(self, tmp_path, monkeypatch): + def test_ref_root_argument_that_is_not_a_directory_is_rejected(self, tmp_path, monkeypatch): + monkeypatch.delenv(REF_ROOT_ENV, raising=False) contract = self._monorepo(tmp_path) - with pytest.raises(RefResolutionError, match="is not a directory"): + with pytest.raises(RefResolutionError, match="ref_root=.* is not a directory"): load_contract(contract, ref_root=tmp_path / "missing") - monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path / "missing")) - with pytest.raises(RefResolutionError, match=f"{REF_ROOT_ENV}=.* is not a directory"): - load_contract(contract) + + def test_ref_root_argument_wins_over_a_usable_env_var(self, tmp_path, monkeypatch): + contract = self._monorepo(tmp_path) + monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path)) + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + with pytest.raises(RefResolutionError, match="the ref root must contain the contract"): + load_contract(contract, ref_root=elsewhere) def test_stale_env_var_does_not_break_ref_free_contracts(self, tmp_path, monkeypatch): monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path / "missing")) @@ -314,6 +324,81 @@ def test_url_refs_stay_refused_under_the_opt_in(self, tmp_path, monkeypatch): load_contract(contract) +# --------------------------------------------------------------------------- +# FLUID_REF_ROOT is process-wide: it must not break contracts outside it +# --------------------------------------------------------------------------- + + +class TestEnvRootOutsideTheContract: + """A ``FLUID_REF_ROOT`` set once (a shell, a service container) applies to + every contract the process loads. One that does not contain the contract, + or is not a directory, is ignored for that contract with a WARNING, and + the contract gets the default root. ``ref_root=`` stays strict (above).""" + + @pytest.fixture(autouse=True) + def _fresh_warning_state(self): + loader._NOTED_REF_ROOT_ENV_IGNORED.clear() + yield + loader._NOTED_REF_ROOT_ENV_IGNORED.clear() + + @staticmethod + def _fragment_contract(tmp: Path) -> Path: + """A contract whose only ref stays in its own directory, like the + ``examples/0.7.1/bitcoin-multifile`` contract.""" + proj = tmp / "uploads" / "c1" + _write(proj / "fragments" / "labels.yaml", {"team": "orders"}) + return _contract(proj, "./fragments/labels.yaml") + + @pytest.mark.parametrize("env_root", ["elsewhere", "missing"]) + @pytest.mark.parametrize("entry", ["load_contract", "compile_contract", "load_with_overlay"]) + def test_contract_outside_the_env_root_loads_with_the_default_root( + self, tmp_path, monkeypatch, caplog, entry, env_root + ): + (tmp_path / "elsewhere").mkdir() + contract = self._fragment_contract(tmp_path) + monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path / env_root)) + with caplog.at_level(logging.WARNING, logger="fluid.loader"): + result = getattr(loader, entry)(contract) + assert result["labels"] == {"team": "orders"} + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert [getattr(r, "event", None) for r in warnings] == ["ref_root_env_ignored"] + message = warnings[0].getMessage() + assert f"{REF_ROOT_ENV}=" in message + assert str(contract.resolve().parent) in message + + def test_fallback_is_the_default_root_not_a_wider_one(self, tmp_path, monkeypatch): + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir() + _write(tmp_path / "outside" / "secret.yaml", {"api_key": SECRET}) + contract = _contract(tmp_path / "proj", "../outside/secret.yaml") + monkeypatch.setenv(REF_ROOT_ENV, str(elsewhere)) + with pytest.raises(RefConfinementError, match="escapes the ref root") as exc: + load_contract(contract) + assert Path(exc.value.root) == contract.resolve().parent + assert SECRET not in str(exc.value) + + def test_warning_is_emitted_once_per_contract_and_value(self, tmp_path, monkeypatch, caplog): + (tmp_path / "elsewhere").mkdir() + contract = self._fragment_contract(tmp_path) + monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path / "elsewhere")) + with caplog.at_level(logging.WARNING, logger="fluid.loader"): + load_contract(contract) + load_with_overlay(contract) + compile_contract(contract) + events = [getattr(r, "event", None) for r in caplog.records] + assert events.count("ref_root_env_ignored") == 1 + + def test_env_root_containing_the_contract_still_widens_without_warning( + self, tmp_path, monkeypatch, caplog + ): + _write(tmp_path / "shared" / "policy.yaml", {"classification": "Internal"}) + contract = _contract(tmp_path / "products" / "orders", "../../shared/policy.yaml") + monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path)) + with caplog.at_level(logging.WARNING, logger="fluid.loader"): + assert load_contract(contract)["labels"] == {"classification": "Internal"} + assert not [r for r in caplog.records if r.levelno >= logging.WARNING] + + # --------------------------------------------------------------------------- # The real CLI # --------------------------------------------------------------------------- @@ -385,3 +470,20 @@ def test_validate_accepts_it_under_the_documented_opt_in(self, layout): contract = _write(proj / "contract.fluid.yaml", _VALID_CONTRACT) result = _fluid("validate", str(contract), cwd=proj, env_extra={REF_ROOT_ENV: str(tmp)}) assert result.returncode == 0, result.stdout + result.stderr + + def test_env_root_elsewhere_does_not_break_a_contract_with_local_fragments(self, layout): + """``FLUID_REF_ROOT`` set for another tree (e.g. once, in a service + container) must not fail an unrelated contract whose refs stay in its + own directory.""" + tmp, proj = layout + _write(proj / "fragments" / "labels.yaml", {"team": "orders"}) + contract = _write( + proj / "contract.fluid.yaml", + {**_VALID_CONTRACT, "labels": {"$ref": "./fragments/labels.yaml"}}, + ) + result = _fluid( + "validate", str(contract), cwd=proj, env_extra={REF_ROOT_ENV: str(tmp / "outside")} + ) + out = " ".join((result.stdout + result.stderr).split()) + assert result.returncode == 0, out + assert "contract_load_failed" not in out From 38d3224c22a425415ebfd4433bee2ac88803fcd6 Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 16:31:50 +0200 Subject: [PATCH 3/9] fix(loader): an escape after an ignored FLUID_REF_ROOT says it was ignored When FLUID_REF_ROOT is not a directory, or does not contain the contract, the loader ignores it for that contract and logs ref_root_env_ignored. That warning is deduplicated per process (per contract directory and value). The RefConfinementError that follows still ended with the generic hint "set FLUID_REF_ROOT to that directory", which is wrong advice when the variable is already set. A second load in the same process (a Command Center worker, any library caller) got that wrong advice with no warning at all, and in a service the warning reaches only the server log, never the caller. _effective_ref_root now returns a _RefRoot (ref_root, root_hint, ignored_ref_root_env), and _resolve_refs threads both new fields through nested refs to confine_ref. When the fallback is taken, every escape error says "FLUID_REF_ROOT is set but was ignored for this contract: ". This does not depend on the warning dedupe. RefConfinementError gains ignored_ref_root_env (None unless the variable was ignored), so a library caller can check it without parsing the message. docs/contract-refs.md said a FLUID_REF_ROOT left in the shell "never widens or breaks another contract". That holds only for contracts outside it. Every contract inside it is widened, silently. The docs now say so, and the upgrade note scopes the variable to a single command instead of exporting it. The new attribute is documented. Tests load twice through load_contract, compile_contract and load_with_overlay, for both a missing and a non-containing root, with the escape in a nested fragment. They check that both errors explain the fallback and that only the first load logs. These tests fail on the previous commit and also against a mutant that explains only on the first load. A separate test pins the generic hint and ignored_ref_root_env=None when the variable is unset or applied. --- docs/contract-refs.md | 25 ++++++++-- fluid_build/loader.py | 68 ++++++++++++++++++++++++---- fluid_build/util/ref_confinement.py | 12 +++++ tests/test_loader_ref_confinement.py | 52 +++++++++++++++++++++ 4 files changed, 143 insertions(+), 14 deletions(-) diff --git a/docs/contract-refs.md b/docs/contract-refs.md index b5521bbb..88ec7a70 100644 --- a/docs/contract-refs.md +++ b/docs/contract-refs.md @@ -125,9 +125,16 @@ Rules for the wider root: ``` Refs that stay in that directory resolve as usual, and refs that leave it - fail as escapes. A `FLUID_REF_ROOT` left in your shell, or set once for a - service that also loads uploaded contracts from temp directories, never - widens or breaks another contract. + fail as escapes. The warning is logged once per contract directory and + value in a process, so every such escape error also says that + `FLUID_REF_ROOT` was ignored for this contract, and why. + + For a contract outside `FLUID_REF_ROOT` the variable is ignored, so it + cannot break or widen that contract. Every contract inside it is widened, + with no warning: a value left set in your shell widens every contract under + it, and a service whose upload directory sits under its `FLUID_REF_ROOT` + widens every uploaded contract. Scope the variable to one command instead + of exporting it. - `ref_root=` is set by the caller for one contract, so it is strict: a `ref_root` that is not a directory or does not contain the contract raises `RefResolutionError`, naming `ref_root`. @@ -149,10 +156,12 @@ Set `FLUID_REF_ROOT` (or `ref_root=`) to the repository root, or to the narrowest directory that holds the products and their shared fragments: ```bash -export FLUID_REF_ROOT="$(git rev-parse --show-toplevel)" -fluid validate products/orders/contract.fluid.yaml +FLUID_REF_ROOT="$(git rev-parse --show-toplevel)" fluid validate products/orders/contract.fluid.yaml ``` +Set it per command, as above, rather than with `export`: an exported value +widens every contract under it that you load later in that shell. + Contracts whose refs stay inside their own directory need no change. --- @@ -173,8 +182,14 @@ except RefConfinementError as err: print(err.pointer) # '/exposes/0/policy' print(err.source) # file that contains the ref print(err.root) # the ref root it escaped + print(err.ignored_ref_root_env) # FLUID_REF_ROOT value ignored for this + # contract, or None ``` +`ignored_ref_root_env` is set when `FLUID_REF_ROOT` is set but did not apply +to this contract (it is not a directory, or does not contain the contract). +The root is then the contract's own directory, whatever the environment says. + --- ## OpenAPI fragments inside a bundle diff --git a/fluid_build/loader.py b/fluid_build/loader.py index d6a95cf2..edf07f4c 100644 --- a/fluid_build/loader.py +++ b/fluid_build/loader.py @@ -20,7 +20,7 @@ import os import threading from pathlib import Path -from typing import Any, Dict, List, Mapping, Optional, Set, Tuple, Union +from typing import Any, Dict, List, Mapping, NamedTuple, Optional, Set, Tuple, Union from fluid_build.util.ref_confinement import ( REF_ROOT_ENV, @@ -281,6 +281,8 @@ def _resolve_refs( base_dir: Path, *, ref_root: Optional[Path] = None, + root_hint: str = _REF_ROOT_HINT, + ignored_ref_root_env: Optional[str] = None, _source: Optional[Path] = None, _loc: Tuple[Union[str, int], ...] = (), _seen: Optional[Set[str]] = None, @@ -306,7 +308,10 @@ def _resolve_refs( layer, for callers that widen ``ref_root``. - Circular reference detection (tracks resolved absolute paths) - Depth limit (``_MAX_REF_DEPTH``) to prevent runaway recursion - - Clear error messages naming the ref and its JSON pointer + - Clear error messages naming the ref and its JSON pointer. + ``root_hint`` ends an escape message; ``ignored_ref_root_env`` is + recorded on every :class:`RefConfinementError`. Both come from + :func:`_effective_ref_root` and are held for nested refs too. """ if _depth > _MAX_REF_DEPTH: raise RefResolutionError( @@ -343,7 +348,8 @@ def _resolve_refs( root=root, pointer=format_pointer(_loc), source=_source, - root_hint=_REF_ROOT_HINT, + root_hint=root_hint, + ignored_ref_root_env=ignored_ref_root_env, ) # Defense in depth (F3): system directories stay blocked even when a @@ -404,6 +410,8 @@ def _resolve_refs( resolved, ref_path.parent, ref_root=root, + root_hint=root_hint, + ignored_ref_root_env=ignored_ref_root_env, _source=ref_path, _loc=_pointer_parts(pointer), _seen=_seen, @@ -421,6 +429,8 @@ def _resolve_refs( v, base_dir, ref_root=root, + root_hint=root_hint, + ignored_ref_root_env=ignored_ref_root_env, _source=_source, _loc=(*_loc, k), _seen=_seen, @@ -436,6 +446,8 @@ def _resolve_refs( item, base_dir, ref_root=root, + root_hint=root_hint, + ignored_ref_root_env=ignored_ref_root_env, _source=_source, _loc=(*_loc, i), _seen=_seen, @@ -448,11 +460,23 @@ def _resolve_refs( return obj +class _RefRoot(NamedTuple): + """What :func:`_effective_ref_root` decided; each field is the + :func:`_resolve_refs` keyword argument of the same name.""" + + ref_root: Path + #: Ends an escape error: how to widen the root, or, when + #: ``FLUID_REF_ROOT`` was ignored, that it was and why. + root_hint: str = _REF_ROOT_HINT + #: The ignored ``FLUID_REF_ROOT`` value, else ``None``. + ignored_ref_root_env: Optional[str] = None + + def _effective_ref_root( contract_path: Path, contract: Any, ref_root: Optional[Union[str, Path]], -) -> Path: +) -> _RefRoot: """The directory every ``$ref`` of *contract* must stay inside. Default: the directory of the root contract file (symlinks resolved). @@ -475,6 +499,9 @@ def _effective_ref_root( WARNING, and the contract gets the default root: exactly the root it would get with the variable unset, so the fallback widens nothing and refs that leave the contract's directory still fail, as escapes. + Those escape errors say the variable was ignored and why, on every + load: the WARNING is logged once per process, and in a service it + reaches the server log, not the caller. """ contract_dir = contract_path.resolve().parent if ref_root is not None: @@ -482,7 +509,7 @@ def _effective_ref_root( else: explicit, origin = os.environ.get(REF_ROOT_ENV, "").strip(), REF_ROOT_ENV if not explicit or next(iter_external_refs(contract), None) is None: - return contract_dir + return _RefRoot(contract_dir) root = Path(explicit).expanduser().resolve() if not root.is_dir(): problem = f"{origin}={explicit!r} is not a directory (resolved to {root})" @@ -492,11 +519,20 @@ def _effective_ref_root( f"(resolved to {root}); the ref root must contain the contract" ) else: - return root + return _RefRoot(root) if origin != REF_ROOT_ENV: raise RefResolutionError(problem) _note_ref_root_env_ignored(contract_dir, explicit, problem) - return contract_dir + return _RefRoot( + contract_dir, + root_hint=( + f"{REF_ROOT_ENV} is set but was ignored for this contract: {problem}. " + f"To compose fragments from a wider tree, set {REF_ROOT_ENV} to a " + f"directory that contains the contract or pass ref_root= to the " + f"loader; see docs/contract-refs.md." + ), + ignored_ref_root_env=explicit, + ) #: (contract directory, FLUID_REF_ROOT value) pairs already reported by @@ -560,7 +596,14 @@ def compile_contract( log.info("compile_start", extra={"path": str(p)}) root = _effective_ref_root(p, contract, ref_root) - compiled = _resolve_refs(contract, p.parent, ref_root=root, _source=p) + compiled = _resolve_refs( + contract, + p.parent, + ref_root=root.ref_root, + root_hint=root.root_hint, + ignored_ref_root_env=root.ignored_ref_root_env, + _source=p, + ) log.info("compile_done", extra={"path": str(p)}) return compiled @@ -848,7 +891,14 @@ def load_contract( if resolve_refs: source = p.resolve() root = _effective_ref_root(p, contract, ref_root) - contract = _resolve_refs(contract, source.parent, ref_root=root, _source=source) + contract = _resolve_refs( + contract, + source.parent, + ref_root=root.ref_root, + root_hint=root.root_hint, + ignored_ref_root_env=root.ignored_ref_root_env, + _source=source, + ) return contract diff --git a/fluid_build/util/ref_confinement.py b/fluid_build/util/ref_confinement.py index 7f3c796a..aef20eda 100644 --- a/fluid_build/util/ref_confinement.py +++ b/fluid_build/util/ref_confinement.py @@ -93,6 +93,12 @@ class RefConfinementError(RefResolutionError): ``contract_load_failed`` path) handles it unchanged. The attributes let a caller such as the Command Center report the offending ref without parsing the message. + + ``ignored_ref_root_env`` is the ``FLUID_REF_ROOT`` value the loader + ignored for this contract (it was not a directory, or did not contain the + contract), or ``None`` when the variable was unset or applied. A non-None + value means the variable did NOT widen this contract's root, whatever the + caller's environment says. """ def __init__( @@ -103,12 +109,14 @@ def __init__( pointer: str, source: Optional[str] = None, root: Optional[str] = None, + ignored_ref_root_env: Optional[str] = None, ) -> None: super().__init__(message) self.ref = ref self.pointer = pointer self.source = source self.root = root + self.ignored_ref_root_env = ignored_ref_root_env def format_pointer(parts: Sequence[Union[str, int]]) -> str: @@ -139,6 +147,7 @@ def confine_ref( pointer: str = "", source: Optional[Path] = None, root_hint: str = "", + ignored_ref_root_env: Optional[str] = None, ) -> Path: """Return the resolved target of an external ``$ref``, or refuse it. @@ -152,6 +161,8 @@ def confine_ref( pointer: JSON pointer of the ``$ref`` node inside *source*. source: The file containing the ref, for the message. root_hint: Appended to the escape message (how to widen the root). + ignored_ref_root_env: Recorded on the error unchanged: the + ``FLUID_REF_ROOT`` value the caller ignored for this document. Raises: RefConfinementError: the ref is a URL, an absolute path, escapes @@ -166,6 +177,7 @@ def _refuse(reason: str) -> RefConfinementError: pointer=pointer, source=str(source) if source is not None else None, root=str(root) if root is not None else None, + ignored_ref_root_env=ignored_ref_root_env, ) if file_part.startswith("//") or ( diff --git a/tests/test_loader_ref_confinement.py b/tests/test_loader_ref_confinement.py index 40fa19eb..19289258 100644 --- a/tests/test_loader_ref_confinement.py +++ b/tests/test_loader_ref_confinement.py @@ -377,6 +377,58 @@ def test_fallback_is_the_default_root_not_a_wider_one(self, tmp_path, monkeypatc assert Path(exc.value.root) == contract.resolve().parent assert SECRET not in str(exc.value) + @pytest.mark.parametrize("env_root", ["elsewhere", "missing"]) + @pytest.mark.parametrize("entry", ["load_contract", "compile_contract", "load_with_overlay"]) + def test_every_escape_error_explains_the_ignored_env_root( + self, tmp_path, monkeypatch, caplog, entry, env_root + ): + """The WARNING is logged once per process, and a service's log is not + its caller. So each escape error says the variable was ignored and + why, and does not tell the user to set a variable that is set.""" + (tmp_path / "elsewhere").mkdir() + _write(tmp_path / "outside" / "secret.yaml", {"api_key": SECRET}) + proj = tmp_path / "proj" + # The escape is in a fragment: the explanation follows nested refs. + _write(proj / "fragments" / "labels.yaml", {"$ref": "../../outside/secret.yaml"}) + contract = _contract(proj, "./fragments/labels.yaml") + value = str(tmp_path / env_root) + monkeypatch.setenv(REF_ROOT_ENV, value) + errors = [] + with caplog.at_level(logging.WARNING, logger="fluid.loader"): + for _ in range(2): + with pytest.raises(RefConfinementError, match="escapes the ref root") as exc: + getattr(loader, entry)(contract) + errors.append(exc.value) + # The second load logged nothing, so its error is the only explanation. + events = [getattr(r, "event", None) for r in caplog.records] + assert events.count("ref_root_env_ignored") == 1 + for err in errors: + message = str(err) + assert f"{REF_ROOT_ENV} is set but was ignored for this contract" in message + assert f"{REF_ROOT_ENV}={value!r}" in message + reason = "is not a directory" if env_root == "missing" else "is outside" + assert reason in message + assert f"set {REF_ROOT_ENV} to that directory" not in message + assert err.ignored_ref_root_env == value + assert Path(err.root) == contract.resolve().parent + assert SECRET not in message + + def test_escape_without_a_fallback_keeps_the_widening_hint(self, tmp_path, monkeypatch): + _write(tmp_path / "outside" / "secret.yaml", {"api_key": SECRET}) + contract = _contract(tmp_path / "proj", "../outside/secret.yaml") + monkeypatch.delenv(REF_ROOT_ENV, raising=False) + with pytest.raises(RefConfinementError) as unset: + load_contract(contract) + # Set and applied: it widened the root, so it was not ignored. + repo = tmp_path / "proj" + monkeypatch.setenv(REF_ROOT_ENV, str(repo)) + with pytest.raises(RefConfinementError) as applied: + load_contract(contract) + for err in (unset.value, applied.value): + assert f"set {REF_ROOT_ENV} to that directory" in str(err) + assert "was ignored" not in str(err) + assert err.ignored_ref_root_env is None + def test_warning_is_emitted_once_per_contract_and_value(self, tmp_path, monkeypatch, caplog): (tmp_path / "elsewhere").mkdir() contract = self._fragment_contract(tmp_path) From 7d2cd0ccb32e12d92be29d118f20ee3296f5b90b Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 17:07:34 +0200 Subject: [PATCH 4/9] fix(loader): an FLUID_REF_ROOT that cannot be resolved falls back too The fallback for an unusable FLUID_REF_ROOT only covered a value that resolved to something that is not a directory, or does not contain the contract. A value that is more broken raised before the fallback was reached: `~gone/repo` for a user that no longer exists (RuntimeError), a symlink loop (RuntimeError on 3.10/3.12), or a root under a directory the process cannot enter (PermissionError from is_dir). The raw error failed every contract with a file $ref, gave no ref_root_env_ignored warning, never named FLUID_REF_ROOT, and slipped past `except RefResolutionError`. ref_root= raised the same raw errors instead of the documented RefResolutionError. Resolution and the is_dir/containment checks now run under `except (OSError, RuntimeError, ValueError)`; a failure becomes "= cannot be resolved: " and takes the existing path: RefResolutionError for ref_root=, ignored with the warning and the per-error hint for FLUID_REF_ROOT. The ref_root_env_ignored dedupe is now keyed on the value alone, not (contract directory, value). A service that loads each upload from a fresh temp directory added one permanent entry and logged one warning per upload; it now logs once per value and holds one key. Each later escape error still says the variable was ignored and why. --- docs/contract-refs.md | 22 ++++--- fluid_build/loader.py | 60 +++++++++++------- fluid_build/util/ref_confinement.py | 4 +- tests/test_loader_ref_confinement.py | 94 +++++++++++++++++++++++++++- 4 files changed, 145 insertions(+), 35 deletions(-) diff --git a/docs/contract-refs.md b/docs/contract-refs.md index 88ec7a70..942a6465 100644 --- a/docs/contract-refs.md +++ b/docs/contract-refs.md @@ -110,8 +110,10 @@ Rules for the wider root: - It widens the root only for a contract inside it, and only if it is an existing directory. - `FLUID_REF_ROOT` applies to every contract the process loads, so it is - ignored for a contract outside it (or when it is not a directory). That - contract gets the default root, its own directory, and a warning says so: + ignored for a contract outside it, and when it is not a directory or cannot + be resolved at all (a `~user` that no longer exists, a symlink loop, a + parent the process cannot enter). That contract gets the default root, its + own directory, and a warning says so: ```bash FLUID_REF_ROOT=repo fluid validate /tmp/upload-1234/contract.fluid.yaml @@ -121,12 +123,15 @@ Rules for the wider root: ref_root_env_ignored: contract /tmp/upload-1234/contract.fluid.yaml is outside FLUID_REF_ROOT='repo' (…); the ref root must contain the contract. Ignoring it for this contract: its $refs are confined to the contract's own directory - /tmp/upload-1234 (the default). + /tmp/upload-1234 (the default). Logged once per value in this process; a + later contract it is ignored for gets no warning, but its escape errors say + the variable was ignored and why. ``` Refs that stay in that directory resolve as usual, and refs that leave it - fail as escapes. The warning is logged once per contract directory and - value in a process, so every such escape error also says that + fail as escapes. The warning is logged once per value in a process, not + once per contract, so a service that loads each upload from a fresh + directory logs it once. Every such escape error therefore also says that `FLUID_REF_ROOT` was ignored for this contract, and why. For a contract outside `FLUID_REF_ROOT` the variable is ignored, so it @@ -136,8 +141,8 @@ Rules for the wider root: widens every uploaded contract. Scope the variable to one command instead of exporting it. - `ref_root=` is set by the caller for one contract, so it is strict: a - `ref_root` that is not a directory or does not contain the contract raises - `RefResolutionError`, naming `ref_root`. + `ref_root` that cannot be resolved, is not a directory, or does not contain + the contract raises `RefResolutionError`, naming `ref_root`. - A blank `FLUID_REF_ROOT` counts as unset: the default root applies. - It is only consulted when the contract has a ref to another file. - It widens the root and nothing else. URLs and absolute paths are still @@ -187,7 +192,8 @@ except RefConfinementError as err: ``` `ignored_ref_root_env` is set when `FLUID_REF_ROOT` is set but did not apply -to this contract (it is not a directory, or does not contain the contract). +to this contract (it cannot be resolved, is not a directory, or does not +contain the contract). The root is then the contract's own directory, whatever the environment says. --- diff --git a/fluid_build/loader.py b/fluid_build/loader.py index edf07f4c..e47a3222 100644 --- a/fluid_build/loader.py +++ b/fluid_build/loader.py @@ -487,8 +487,8 @@ def _effective_ref_root( The opt-in is only consulted when the contract has an external ref. - The two sources fail differently when the root is unusable (not a - directory, or does not contain the contract): + The two sources fail differently when the root is unusable (cannot be + resolved, is not a directory, or does not contain the contract): * ``ref_root=`` is a choice the caller made for THIS contract, so it is a :class:`RefResolutionError`. @@ -500,8 +500,8 @@ def _effective_ref_root( would get with the variable unset, so the fallback widens nothing and refs that leave the contract's directory still fail, as escapes. Those escape errors say the variable was ignored and why, on every - load: the WARNING is logged once per process, and in a service it - reaches the server log, not the caller. + load: the WARNING is logged once per value per process, and in a + service it reaches the server log, not the caller. """ contract_dir = contract_path.resolve().parent if ref_root is not None: @@ -510,16 +510,24 @@ def _effective_ref_root( explicit, origin = os.environ.get(REF_ROOT_ENV, "").strip(), REF_ROOT_ENV if not explicit or next(iter_external_refs(contract), None) is None: return _RefRoot(contract_dir) - root = Path(explicit).expanduser().resolve() - if not root.is_dir(): - problem = f"{origin}={explicit!r} is not a directory (resolved to {root})" - elif not contract_dir.is_relative_to(root): - problem = ( - f"contract {contract_path} is outside {origin}={explicit!r} " - f"(resolved to {root}); the ref root must contain the contract" - ) - else: - return _RefRoot(root) + try: + # A stale value can fail to resolve at all: ``~gone/repo`` for a + # user that no longer exists, a symlink loop, or a parent the + # process cannot traverse (``is_dir`` does not swallow EACCES). + # Those are unusable roots too, and take the same path below. + root = Path(explicit).expanduser().resolve() + if not root.is_dir(): + problem = f"{origin}={explicit!r} is not a directory (resolved to {root})" + elif not contract_dir.is_relative_to(root): + problem = ( + f"contract {contract_path} is outside {origin}={explicit!r} " + f"(resolved to {root}); the ref root must contain the contract" + ) + else: + return _RefRoot(root) + except (OSError, RuntimeError, ValueError) as exc: + # The problem is followed by ". ..." in the warning and the hint. + problem = f"{origin}={explicit!r} cannot be resolved: {str(exc).rstrip('.')}" if origin != REF_ROOT_ENV: raise RefResolutionError(problem) _note_ref_root_env_ignored(contract_dir, explicit, problem) @@ -535,24 +543,28 @@ def _effective_ref_root( ) -#: (contract directory, FLUID_REF_ROOT value) pairs already reported by -#: :func:`_note_ref_root_env_ignored` in this process — one command loads the -#: same contract several times. Tests reset it with ``.clear()``. -_NOTED_REF_ROOT_ENV_IGNORED: Set[Tuple[str, str]] = set() +#: FLUID_REF_ROOT values already reported by :func:`_note_ref_root_env_ignored` +#: in this process. Keyed on the value alone, not the contract directory: a +#: service loads each upload from a fresh temp directory, so a per-directory +#: key would grow without bound and log once per upload. The escape errors +#: of every later contract carry their own explanation (``root_hint``). +#: Tests reset it with ``.clear()``. +_NOTED_REF_ROOT_ENV_IGNORED: Set[str] = set() _NOTED_REF_ROOT_ENV_IGNORED_LOCK = threading.Lock() def _note_ref_root_env_ignored(contract_dir: Path, value: str, problem: str) -> None: - """WARN, once per (contract directory, value), that ``FLUID_REF_ROOT`` - does not apply to this contract and the default root is used.""" - key = (str(contract_dir), value) + """WARN, once per ``FLUID_REF_ROOT`` value per process, that it does not + apply to this contract and the default root is used.""" with _NOTED_REF_ROOT_ENV_IGNORED_LOCK: - if key in _NOTED_REF_ROOT_ENV_IGNORED: + if value in _NOTED_REF_ROOT_ENV_IGNORED: return - _NOTED_REF_ROOT_ENV_IGNORED.add(key) + _NOTED_REF_ROOT_ENV_IGNORED.add(value) LOG.warning( "ref_root_env_ignored: %s. Ignoring it for this contract: its $refs are " - "confined to the contract's own directory %s (the default).", + "confined to the contract's own directory %s (the default). Logged once " + "per value in this process; a later contract it is ignored for gets no " + "warning, but its escape errors say the variable was ignored and why.", problem, contract_dir, extra={ diff --git a/fluid_build/util/ref_confinement.py b/fluid_build/util/ref_confinement.py index aef20eda..6716f13f 100644 --- a/fluid_build/util/ref_confinement.py +++ b/fluid_build/util/ref_confinement.py @@ -95,8 +95,8 @@ class RefConfinementError(RefResolutionError): parsing the message. ``ignored_ref_root_env`` is the ``FLUID_REF_ROOT`` value the loader - ignored for this contract (it was not a directory, or did not contain the - contract), or ``None`` when the variable was unset or applied. A non-None + ignored for this contract (it could not be resolved, was not a directory, + or did not contain the contract), or ``None`` when the variable was unset or applied. A non-None value means the variable did NOT widen this contract's root, whatever the caller's environment says. """ diff --git a/tests/test_loader_ref_confinement.py b/tests/test_loader_ref_confinement.py index 19289258..5689b1dd 100644 --- a/tests/test_loader_ref_confinement.py +++ b/tests/test_loader_ref_confinement.py @@ -304,6 +304,16 @@ def test_ref_root_argument_that_is_not_a_directory_is_rejected(self, tmp_path, m with pytest.raises(RefResolutionError, match="ref_root=.* is not a directory"): load_contract(contract, ref_root=tmp_path / "missing") + def test_ref_root_argument_that_cannot_be_resolved_is_a_ref_resolution_error( + self, tmp_path, monkeypatch, unresolvable_root + ): + """Not a raw RuntimeError / PermissionError that an + ``except RefResolutionError`` handler would miss.""" + monkeypatch.delenv(REF_ROOT_ENV, raising=False) + contract = self._monorepo(tmp_path) + with pytest.raises(RefResolutionError, match="ref_root=.* cannot be resolved"): + load_contract(contract, ref_root=unresolvable_root) + def test_ref_root_argument_wins_over_a_usable_env_var(self, tmp_path, monkeypatch): contract = self._monorepo(tmp_path) monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path)) @@ -429,7 +439,7 @@ def test_escape_without_a_fallback_keeps_the_widening_hint(self, tmp_path, monke assert "was ignored" not in str(err) assert err.ignored_ref_root_env is None - def test_warning_is_emitted_once_per_contract_and_value(self, tmp_path, monkeypatch, caplog): + def test_warning_is_emitted_once_per_value(self, tmp_path, monkeypatch, caplog): (tmp_path / "elsewhere").mkdir() contract = self._fragment_contract(tmp_path) monkeypatch.setenv(REF_ROOT_ENV, str(tmp_path / "elsewhere")) @@ -440,6 +450,88 @@ def test_warning_is_emitted_once_per_contract_and_value(self, tmp_path, monkeypa events = [getattr(r, "event", None) for r in caplog.records] assert events.count("ref_root_env_ignored") == 1 + def test_uploads_in_fresh_directories_warn_once_and_stay_bounded( + self, tmp_path, monkeypatch, caplog + ): + """A service with ``FLUID_REF_ROOT`` set once loads each upload from a + fresh temp directory. One WARNING per value, and one remembered key, + however many uploads; each escape error still explains itself.""" + (tmp_path / "elsewhere").mkdir() + _write(tmp_path / "outside" / "secret.yaml", {"api_key": SECRET}) + value = str(tmp_path / "elsewhere") + monkeypatch.setenv(REF_ROOT_ENV, value) + with caplog.at_level(logging.WARNING, logger="fluid.loader"): + for i in range(5): + upload = tmp_path / "uploads" / f"upload-{i}" + _write(upload / "fragments" / "labels.yaml", {"team": f"t{i}"}) + ok = _contract(upload, "./fragments/labels.yaml") + assert load_contract(ok)["labels"] == {"team": f"t{i}"} + bad = _contract(upload, "../../../outside/secret.yaml", name="bad.yaml") + with pytest.raises(RefConfinementError) as exc: + load_contract(bad) + assert f"{REF_ROOT_ENV} is set but was ignored for this contract" in str(exc.value) + assert exc.value.ignored_ref_root_env == value + events = [getattr(r, "event", None) for r in caplog.records] + assert events.count("ref_root_env_ignored") == 1 + assert len(loader._NOTED_REF_ROOT_ENV_IGNORED) == 1 + + def test_env_root_that_cannot_be_resolved_falls_back( + self, tmp_path, monkeypatch, caplog, unresolvable_root + ): + """A stale value can be broken past "not a directory": resolving it + raises (RuntimeError / PermissionError). It must still be ignored + with the warning, not fail every contract with a file ``$ref``.""" + value = unresolvable_root + contract = self._fragment_contract(tmp_path) + monkeypatch.setenv(REF_ROOT_ENV, value) + for entry in ("load_contract", "compile_contract", "load_with_overlay"): + loader._NOTED_REF_ROOT_ENV_IGNORED.clear() + caplog.clear() + with caplog.at_level(logging.WARNING, logger="fluid.loader"): + result = getattr(loader, entry)(contract) + assert result["labels"] == {"team": "orders"} + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert [getattr(r, "event", None) for r in warnings] == ["ref_root_env_ignored"] + assert f"{REF_ROOT_ENV}={value!r}" in warnings[0].getMessage() + + def test_unresolvable_env_root_escape_error_explains_it( + self, tmp_path, monkeypatch, unresolvable_root + ): + value = unresolvable_root + _write(tmp_path / "outside" / "secret.yaml", {"api_key": SECRET}) + contract = _contract(tmp_path / "proj", "../outside/secret.yaml") + monkeypatch.setenv(REF_ROOT_ENV, value) + with pytest.raises(RefConfinementError, match="escapes the ref root") as exc: + load_contract(contract) + assert f"{REF_ROOT_ENV} is set but was ignored for this contract" in str(exc.value) + assert exc.value.ignored_ref_root_env == value + assert Path(exc.value.root) == contract.resolve().parent + assert SECRET not in str(exc.value) + + +@pytest.fixture(params=["unknown_user", "symlink_loop", "untraversable_parent"]) +def unresolvable_root(request, tmp_path) -> str: + """A ref-root value whose resolution raises (RuntimeError, + PermissionError) rather than returning a path that merely is not a + directory.""" + kind = request.param + if kind == "unknown_user": + if os.name == "nt": # expanduser guesses a sibling profile dir there + pytest.skip("~user expansion does not fail on Windows") + return "~nosuchuser-fluid-ref-root-xyz/repo" + if kind == "symlink_loop": + _symlink(tmp_path / "loopA", tmp_path / "loopB", is_dir=True) + _symlink(tmp_path / "loopB", tmp_path / "loopA", is_dir=True) + return str(tmp_path / "loopA") + if os.name == "nt" or (hasattr(os, "geteuid") and os.geteuid() == 0): + pytest.skip("chmod 000 does not deny traversal here") + locked = tmp_path / "locked" + (locked / "inner").mkdir(parents=True) + locked.chmod(0) + # Restore traversal so tmp_path cleanup can remove it. + request.addfinalizer(lambda: locked.chmod(0o700)) + return str(locked / "inner") + def test_env_root_containing_the_contract_still_widens_without_warning( self, tmp_path, monkeypatch, caplog ): From 87251e1a2d16853cf90388b84b40cd0e294490c7 Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:11:34 +0200 Subject: [PATCH 5/9] chore(secrets): mark the leak-test sentinel as a test value, and follow AGENTS.md's moved line detect-secrets in Lint & Format flagged SECRET = "SUPER-SECRET-VALUE" in the confinement tests. It is the sentinel the tests assert never reaches the output, not a credential, so it carries the repo's inline "pragma: allowlist secret". The hook also moved one known AGENTS.md entry in .secrets.baseline by a line, because this branch adds a line above it; committing that is what the hook asks for. --- .secrets.baseline | 4 ++-- tests/test_loader_ref_confinement.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 13d9a9a0..5891e9cd 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -157,7 +157,7 @@ "filename": "AGENTS.md", "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", "is_verified": false, - "line_number": 596 + "line_number": 597 } ], "Jenkinsfile": [ @@ -2154,5 +2154,5 @@ } ] }, - "generated_at": "2026-09-28T10:03:17Z" + "generated_at": "2026-10-02T17:10:21Z" } diff --git a/tests/test_loader_ref_confinement.py b/tests/test_loader_ref_confinement.py index 5689b1dd..9a931d9e 100644 --- a/tests/test_loader_ref_confinement.py +++ b/tests/test_loader_ref_confinement.py @@ -43,7 +43,7 @@ load_with_overlay, ) -SECRET = "SUPER-SECRET-VALUE" +SECRET = "SUPER-SECRET-VALUE" # pragma: allowlist secret def _write(path: Path, data) -> Path: From fa0ff2b00264b64b3ad6f958e83b8cc58074be7e Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:25:40 +0200 Subject: [PATCH 6/9] fix(loader): a ref root that is a symlink loop reads "cannot be resolved" on Python 3.13+ too Python 3.13 stopped raising RuntimeError from a non-strict Path.resolve() on a symlink loop; it returns the path instead. On 3.13 and 3.14 a looping ref_root therefore fell through to "is not a directory", and the symlink_loop case of test_ref_root_argument_that_cannot_be_resolved_is_a_ref_resolution_error failed in CI (3.10 and 3.12 passed). When the non-strict resolution is not a directory, the root is now resolved again strictly: a loop or an untraversable parent raises into the existing "cannot be resolved" path on every version, and a missing directory keeps "is not a directory". Verified on 3.10, 3.12, 3.13 and 3.14; the 3.13 test fails without the change. --- fluid_build/loader.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/fluid_build/loader.py b/fluid_build/loader.py index e47a3222..0e500f44 100644 --- a/fluid_build/loader.py +++ b/fluid_build/loader.py @@ -517,6 +517,15 @@ def _effective_ref_root( # Those are unusable roots too, and take the same path below. root = Path(explicit).expanduser().resolve() if not root.is_dir(): + # Python 3.13 stopped raising from a non-strict resolve() on a + # symlink loop (it returns the path instead), so ask strictly: a + # loop or an untraversable parent then reads "cannot be resolved" + # on every version, while a plain missing directory stays + # "is not a directory". + try: + Path(explicit).expanduser().resolve(strict=True) + except FileNotFoundError: + pass problem = f"{origin}={explicit!r} is not a directory (resolved to {root})" elif not contract_dir.is_relative_to(root): problem = ( From cbcff2f3ba3286b378837b36585cfb098c91d7b1 Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 19:27:09 +0200 Subject: [PATCH 7/9] fix(tests): the env-root widening test runs again, instead of hiding inside a fixture test_env_root_containing_the_contract_still_widens_without_warning sat after the return of the module-level unresolvable_root fixture, which had been added between the class's methods, so pytest never collected it (reported by GitHub code quality as unreachable code). The fixture now sits above the class; the test is collected again and passes on 3.10, 3.12 and 3.13. --- tests/test_loader_ref_confinement.py | 48 ++++++++++++++-------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/tests/test_loader_ref_confinement.py b/tests/test_loader_ref_confinement.py index 9a931d9e..15a63342 100644 --- a/tests/test_loader_ref_confinement.py +++ b/tests/test_loader_ref_confinement.py @@ -339,6 +339,30 @@ def test_url_refs_stay_refused_under_the_opt_in(self, tmp_path, monkeypatch): # --------------------------------------------------------------------------- +@pytest.fixture(params=["unknown_user", "symlink_loop", "untraversable_parent"]) +def unresolvable_root(request, tmp_path) -> str: + """A ref-root value whose resolution raises (RuntimeError, + PermissionError) rather than returning a path that merely is not a + directory.""" + kind = request.param + if kind == "unknown_user": + if os.name == "nt": # expanduser guesses a sibling profile dir there + pytest.skip("~user expansion does not fail on Windows") + return "~nosuchuser-fluid-ref-root-xyz/repo" + if kind == "symlink_loop": + _symlink(tmp_path / "loopA", tmp_path / "loopB", is_dir=True) + _symlink(tmp_path / "loopB", tmp_path / "loopA", is_dir=True) + return str(tmp_path / "loopA") + if os.name == "nt" or (hasattr(os, "geteuid") and os.geteuid() == 0): + pytest.skip("chmod 000 does not deny traversal here") + locked = tmp_path / "locked" + (locked / "inner").mkdir(parents=True) + locked.chmod(0) + # Restore traversal so tmp_path cleanup can remove it. + request.addfinalizer(lambda: locked.chmod(0o700)) + return str(locked / "inner") + + class TestEnvRootOutsideTheContract: """A ``FLUID_REF_ROOT`` set once (a shell, a service container) applies to every contract the process loads. One that does not contain the contract, @@ -508,30 +532,6 @@ def test_unresolvable_env_root_escape_error_explains_it( assert Path(exc.value.root) == contract.resolve().parent assert SECRET not in str(exc.value) - -@pytest.fixture(params=["unknown_user", "symlink_loop", "untraversable_parent"]) -def unresolvable_root(request, tmp_path) -> str: - """A ref-root value whose resolution raises (RuntimeError, - PermissionError) rather than returning a path that merely is not a - directory.""" - kind = request.param - if kind == "unknown_user": - if os.name == "nt": # expanduser guesses a sibling profile dir there - pytest.skip("~user expansion does not fail on Windows") - return "~nosuchuser-fluid-ref-root-xyz/repo" - if kind == "symlink_loop": - _symlink(tmp_path / "loopA", tmp_path / "loopB", is_dir=True) - _symlink(tmp_path / "loopB", tmp_path / "loopA", is_dir=True) - return str(tmp_path / "loopA") - if os.name == "nt" or (hasattr(os, "geteuid") and os.geteuid() == 0): - pytest.skip("chmod 000 does not deny traversal here") - locked = tmp_path / "locked" - (locked / "inner").mkdir(parents=True) - locked.chmod(0) - # Restore traversal so tmp_path cleanup can remove it. - request.addfinalizer(lambda: locked.chmod(0o700)) - return str(locked / "inner") - def test_env_root_containing_the_contract_still_widens_without_warning( self, tmp_path, monkeypatch, caplog ): From 49c8f9efe7f6111d39a8e7850baf8f010389b3c1 Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 20:12:45 +0200 Subject: [PATCH 8/9] docs: describe the threat model, not a product The rationale named one downstream service as the system these reads were found in. The engine's docs and docstrings now describe the case generically: services, CI jobs and shared hosts that run contracts other people wrote. --- docs/contract-refs.md | 5 ++--- fluid_build/util/ref_confinement.py | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/contract-refs.md b/docs/contract-refs.md index 942a6465..9928af7c 100644 --- a/docs/contract-refs.md +++ b/docs/contract-refs.md @@ -72,9 +72,8 @@ the file it was written in: `fluid validate` exits 1 and `fluid bundle` exits 2. The check runs before the target is opened, so the error is the same whether or not the target exists. -**Why.** Contracts are often untrusted input: a platform such as the FLUID -Command Center runs `fluid validate` and `fluid bundle` on contracts its users -upload. Without the root, a contract could compose any YAML or JSON file the +**Why.** Contracts are often untrusted input: a service or CI job may run +`fluid validate` and `fluid bundle` on contracts its users upload. Without the root, a contract could compose any YAML or JSON file the process can read into itself, and `fluid bundle` would print it back. --- diff --git a/fluid_build/util/ref_confinement.py b/fluid_build/util/ref_confinement.py index 6716f13f..f6efc633 100644 --- a/fluid_build/util/ref_confinement.py +++ b/fluid_build/util/ref_confinement.py @@ -14,7 +14,7 @@ """The one confinement check every external ``$ref`` goes through. -A contract is untrusted input: the Command Center runs ``fluid validate`` / +A contract is untrusted input: a service runs ``fluid validate`` / ``plan`` / ``bundle`` on contracts users upload. Composing a file into the contract and then echoing the contract back (``bundle`` prints it, ``validate`` quotes it in errors) turns an unconfined ``$ref`` into a read of any @@ -91,8 +91,8 @@ class RefConfinementError(RefResolutionError): Subclasses :class:`RefResolutionError`, so every existing ``except RefResolutionError`` (``fluid bundle``, the contract loader's ``contract_load_failed`` path) handles it unchanged. The attributes let a - caller such as the Command Center report the offending ref without - parsing the message. + caller that runs other people's contracts report the offending ref + without parsing the message. ``ignored_ref_root_env`` is the ``FLUID_REF_ROOT`` value the loader ignored for this contract (it could not be resolved, was not a directory, From 853a8f5dc64de5a79c9962070d2d6e728d01de3d Mon Sep 17 00:00:00 2001 From: fas89 <50082482+fas89@users.noreply.github.com> Date: Fri, 2 Oct 2026 20:31:32 +0200 Subject: [PATCH 9/9] style(loader): say in the except clause why a missing ref root is let through GitHub code quality flagged the FileNotFoundError 'pass' added for the Python 3.13 symlink-loop fix as an empty except without an explanation. --- fluid_build/loader.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fluid_build/loader.py b/fluid_build/loader.py index 0e500f44..40468b02 100644 --- a/fluid_build/loader.py +++ b/fluid_build/loader.py @@ -525,6 +525,8 @@ def _effective_ref_root( try: Path(explicit).expanduser().resolve(strict=True) except FileNotFoundError: + # A plain missing directory is reported as "not a directory" + # just below; only a loop or a permission failure raises on. pass problem = f"{origin}={explicit!r} is not a directory (resolved to {root})" elif not contract_dir.is_relative_to(root):