From 76b10b5efe70820cdeb546f3c07e1686d57179c6 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:43:44 +0200 Subject: [PATCH 1/6] feat(validate): `fluid validate .tgz --strict` fails on an overlay patch the bundle recorded A bundle is never re-overlaid, so `fluid validate .tgz` could not see an overlay patch that landed on another entry (overlay_patch_retargeted): only `fluid bundle --env` logged it, and pipeline stage 2, which runs exactly `fluid validate --env --strict`, passed. `fluid bundle` now merges the overlay inside collect_overlay_warnings(), says each warning once, and records the messages in MANIFEST.json as source.overlayWarnings, only when there are any. The source block is outside the merkle root, so the digest still names the contract alone. Validate reports each one as a warning finding (overlay, MANIFEST.json, OVERLAY-WARNING), so --strict fails on it, and --format json and --report list it. A bundle that records none, one built before this change included, validates as before. The bundle path now prints what the contract path prints. A passing bundle lists its warnings, not only their count. --quiet, which printed nothing, prints ERROR: lines, a WARNING: line for a recorded overlay warning, and under --strict an ERROR (--strict): line for each warning. When warnings alone fail --strict, it says so. Borrowed: the split SLSA Provenance v1 makes between the artifact digest (subject) and run metadata (runDetails). The MANIFEST source block already has that shape. Not SARIF, which is a full diagnostics format, for one list of strings in an advisory block. --- fluid_build/cli/bundle.py | 40 ++++++-- fluid_build/cli/validate.py | 104 ++++++++++++++++---- fluid_build/forge/core/bundle.py | 44 +++++++-- tests/test_overlay_keyed_merge.py | 156 ++++++++++++++++++++++++++++++ 4 files changed, 308 insertions(+), 36 deletions(-) diff --git a/fluid_build/cli/bundle.py b/fluid_build/cli/bundle.py index 24b539ce..69f6208b 100644 --- a/fluid_build/cli/bundle.py +++ b/fluid_build/cli/bundle.py @@ -42,7 +42,12 @@ except Exception: # pragma: no cover yaml = None -from ..loader import RefResolutionError, compile_contract, load_with_overlay +from ..loader import ( + RefResolutionError, + collect_overlay_warnings, + compile_contract, + load_with_overlay, +) from ..observability.tracing import traced_stage COMMAND = "bundle" @@ -386,10 +391,14 @@ def run(args: argparse.Namespace, logger: logging.Logger) -> int: # overlay on top when requested, with the same loud notice as every # other stage when ``--env`` names an overlay that does not exist: a # bundle is the root of trust for stages 2-9, so a silently - # un-overlaid bundle poisons every one of them. - compiled, overlay_path = materialize_contract( - contract_path, env, logger, compile_logger=_compile_logger - ) + # un-overlaid bundle poisons every one of them. A positional overlay + # patch that lands on another entry is collected, not only logged: a + # bundle is never re-overlaid, so the tgz MANIFEST records it for + # ``fluid validate --strict`` (pipeline stage 2) to fail on. + with collect_overlay_warnings() as overlay_warnings: + compiled, overlay_path = materialize_contract( + contract_path, env, logger, compile_logger=_compile_logger + ) except FileNotFoundError as e: sys.stderr.write(f"❌ File not found: {e}\n") @@ -401,6 +410,10 @@ def run(args: argparse.Namespace, logger: logging.Logger) -> int: sys.stderr.write(f"❌ Compilation failed: {e}\n") return 1 + # The loader logs a warning the block took at DEBUG only: say it here, once. + for warning in overlay_warnings: + logger.warning("%s", warning.message, extra={"event": warning.event}) + # Feedback: tell the user if the contract had nothing to bundle # (suppressed for --format tgz — the tgz has fragments regardless of refs) if not has_refs and not env and fmt != "tgz": @@ -436,10 +449,16 @@ def run(args: argparse.Namespace, logger: logging.Logger) -> int: # anchored at the contract's directory, as it is when the contract # itself is the input) and which environment was applied (so # ``plan`` / ``apply`` / ``verify --env X`` can refuse a bundle built - # for a different one). Recorded in MANIFEST.json, outside the - # merkle root: the digest still names the contract alone. + # for a different one), with the overlay warnings the merge raised (so + # ``fluid validate --strict`` fails on them). Recorded in + # MANIFEST.json, outside the merkle root: the digest still names the + # contract alone. source = make_bundle_source( - Path(contract_path), Path(out), env=env, overlay_path=overlay_path + Path(contract_path), + Path(out), + env=env, + overlay_path=overlay_path, + overlay_warnings=[warning.message for warning in overlay_warnings], ) try: digest = build_bundle_tgz( @@ -457,6 +476,11 @@ def run(args: argparse.Namespace, logger: logging.Logger) -> int: if env: applied = source.get("overlay") or "none (base contract)" sys.stderr.write(f" env: {env} (overlay: {applied})\n") + if overlay_warnings: + sys.stderr.write( + f" overlay warnings: {len(overlay_warnings)} recorded in the bundle; " + "`fluid validate --strict` fails on it\n" + ) # ── Optional: Sigstore cosign keyless signing ───────────────── # Runs after tgz emission so the digest is already stable. A diff --git a/fluid_build/cli/validate.py b/fluid_build/cli/validate.py index 8e906a5f..047d6359 100644 --- a/fluid_build/cli/validate.py +++ b/fluid_build/cli/validate.py @@ -1274,20 +1274,20 @@ def _output_text_results(result: ValidationResult, args, logger: logging.Logger) if has_errors or treat_warnings_as_errors: # Under --quiet the warnings were not listed, errors or not. if treat_warnings_as_errors and (args.quiet or not has_errors): - _explain_strict_failure(result, args) + _explain_strict_failure(result.warnings, args.quiet) return 1 else: return 0 -def _explain_strict_failure(result: ValidationResult, args: Any) -> None: +def _explain_strict_failure(warnings: List[str], quiet: bool) -> None: """Say why ``--strict`` exits 1: after a summary that reads valid, or under ``--quiet``. ``--quiet`` prints errors only, so there each warning is printed as the error ``--strict`` made it: a count alone does not say what to fix. """ - if args.quiet: - for warning in result.warnings: + if quiet: + for warning in warnings: _cprint_literal(f"ERROR (--strict): {warning}") else: cprint("❌ Validation failed: warnings treated as errors (--strict)") @@ -1506,13 +1506,15 @@ def _run_bundle_validation( report.issues.insert(0, env_issue) report.status = "fail" - # Contract-level rules (sovereignty, agent policy, binding prerequisites, + # The overlay warnings ``fluid bundle --env`` recorded, then the + # contract-level rules (sovereignty, agent policy, binding prerequisites, # packaging, semantics, composition, plugin validators) on the resolved - # contract the bundle carries — the same rules the contract path runs. + # contract the bundle carries — what the contract path reports. # Skipped when the MANIFEST gate already failed: there is no trustworthy # contract to run them on, and the tamper finding is the answer. manifest_ok = not any(i.code == "MANIFEST-TAMPER" for i in report.issues) if manifest_ok and not (fail_fast and report.status == "fail"): + report.issues.extend(_bundle_overlay_issues(tgz_path)) report.issues.extend(_bundle_contract_rule_issues(tgz_path, args, schema_manager, logger)) has_error = any(i.severity == "error" for i in report.issues) has_warning = any(i.severity == "warning" for i in report.issues) @@ -1525,7 +1527,9 @@ def _run_bundle_validation( if out_format == "json": _cprint_json(json.dumps(report.to_dict(), indent=2)) - elif not quiet: + elif quiet: + _print_quiet_bundle_issues(report.issues, strict) + else: status_icon = "✅" if report.status == "pass" else "❌" cprint(f"{status_icon} Bundle {report.status}: {tgz_path}") cprint(f" digest: {report.bundle_digest}") @@ -1535,17 +1539,14 @@ def _run_bundle_validation( f"({s.get('error', 0)} error, {s.get('warning', 0)} warning, " f"{s.get('info', 0)} info)" ) - if verbose or report.status == "fail": - for issue in report.issues: - loc = "" - if issue.line is not None: - loc = f" (L{issue.line}" - if issue.column is not None: - loc += f":C{issue.column}" - loc += ")" - _cprint_literal( - f" [{issue.severity}] {issue.validator}: {issue.file}{loc}: {issue.message}" - ) + # A bundle that passes still lists its warnings, as a contract file does. + for issue in report.issues: + if verbose or report.status == "fail" or issue.severity == "warning": + _cprint_literal(f" [{issue.severity}] {_bundle_issue_text(issue)}") + if strict and not any(issue.severity == "error" for issue in report.issues): + warnings = [_bundle_issue_text(i) for i in report.issues if i.severity == "warning"] + if warnings: + _explain_strict_failure(warnings, quiet=False) # Structured report file — atomic write. if report_path: @@ -1564,6 +1565,73 @@ def _run_bundle_validation( return 0 if report.status == "pass" else 1 +#: The report code of an overlay warning ``fluid bundle --env`` recorded. +_OVERLAY_WARNING_CODE = "OVERLAY-WARNING" + + +def _bundle_issue_text(issue: Any) -> str: + """One bundle finding as the report lists it: ``validator: file (Lline:Ccol): message``.""" + loc = "" + if issue.line is not None: + loc = f" (L{issue.line}" + if issue.column is not None: + loc += f":C{issue.column}" + loc += ")" + return f"{issue.validator}: {issue.file}{loc}: {issue.message}" + + +def _print_quiet_bundle_issues(issues: List[Any], strict: bool) -> None: + """``--quiet`` on a bundle: what it prints for a contract file. + + Each error; under ``--strict`` each warning, as the error it made + (:func:`_explain_strict_failure`); without it, the overlay warnings + ``fluid bundle --env`` recorded, which ``fluid validate --env + --quiet`` shows too (:func:`_print_quiet_overlay_warnings`). + """ + for issue in issues: + if issue.severity == "error": + _cprint_literal(f"ERROR: {_bundle_issue_text(issue)}") + warnings = [issue for issue in issues if issue.severity == "warning"] + if strict: + _explain_strict_failure([_bundle_issue_text(issue) for issue in warnings], quiet=True) + return + for issue in warnings: + if issue.code == _OVERLAY_WARNING_CODE: + _cprint_literal(f"WARNING: {_bundle_issue_text(issue)}") + + +def _bundle_overlay_issues(tgz_path: Path) -> list: + """The overlay warnings ``fluid bundle --env`` recorded, as report findings. + + A bundle is never re-overlaid, so validating it cannot see a positional + overlay patch the merge landed on another entry: ``fluid bundle`` records + the warning in the MANIFEST ``source`` block, and it is reported here as a + warning, as ``fluid validate --env`` reports it, so + ``--strict`` (pipeline stage 2) fails on it. A bundle that records none, + one built before the record existed included, has none to report. + """ + import tarfile + + from fluid_build.forge.core.bundle import bundle_overlay_warnings, read_bundle_source + from fluid_build.forge.core.validators import ValidationIssue + + try: + source = read_bundle_source(tgz_path) + except (tarfile.TarError, OSError, ValueError): + # Unreadable: the MANIFEST gate before this reports that bundle. + return [] + return [ + ValidationIssue( + file="MANIFEST.json", + validator="overlay", + severity="warning", + message=warning, + code=_OVERLAY_WARNING_CODE, + ) + for warning in bundle_overlay_warnings(source) + ] + + def _bundle_env_issue(tgz_path: Path, env: Optional[str], logger: logging.Logger): """``--env`` against the env the bundle was built for, as a report finding. diff --git a/fluid_build/forge/core/bundle.py b/fluid_build/forge/core/bundle.py index 316a6b55..d7b2944c 100644 --- a/fluid_build/forge/core/bundle.py +++ b/fluid_build/forge/core/bundle.py @@ -54,7 +54,7 @@ import unicodedata from importlib import import_module from pathlib import Path, PurePosixPath, PureWindowsPath -from typing import Any, Dict, Mapping, Optional, Tuple +from typing import Any, Dict, List, Mapping, Optional, Sequence, Tuple import yaml @@ -184,11 +184,12 @@ def build_manifest( reproduce with shell tools (``sha256sum | sort | sha256sum``). ``source`` (optional) is the provenance block :func:`make_bundle_source` - builds: where the source contract lives relative to the bundle, and which - environment overlay was applied. It is metadata about the bundle, not - bundle content, so it is deliberately OUTSIDE the merkle root — the - digest still identifies the resolved contract alone, and two bundles of - the same contract written to different directories keep one digest. + builds: where the source contract lives relative to the bundle, which + environment overlay was applied, and the overlay warnings its merge raised. + It is metadata about the bundle, not bundle content, so it is deliberately + OUTSIDE the merkle root — the digest still identifies the resolved contract + alone, and two bundles of the same contract written to different + directories keep one digest. """ per_file: Dict[str, str] = {} for path in sorted(files.keys()): @@ -406,6 +407,7 @@ def make_bundle_source( *, env: Optional[str] = None, overlay_path: Optional[Path] = None, + overlay_warnings: Sequence[str] = (), ) -> Dict[str, Any]: """Build the MANIFEST ``source`` block for a bundle written to ``out_path``. @@ -415,11 +417,15 @@ def make_bundle_source( an absolute path would differ per CI agent. ``env`` is the ``--env`` the bundle was built with (``None`` when none was given) and ``overlay`` the overlay file actually merged, relative to the contract's directory - (``None`` when no overlay was applied). + (``None`` when no overlay was applied). ``overlayWarnings``, present only + when there are any, lists the overlay merge warnings ``fluid validate + --strict`` fails on (:func:`fluid_build.loader.collect_overlay_warnings`): + a bundle is never re-overlaid, so validating it cannot see them otherwise. Downstream stages use it to anchor relative ``binding.location.path`` - values at the source contract's directory and to refuse an ``--env`` that - disagrees with the environment the bundle was built for. + values at the source contract's directory, to refuse an ``--env`` that + disagrees with the environment the bundle was built for, and to report + the overlay warnings (:func:`bundle_overlay_warnings`). """ contract_abs = Path(contract_path).resolve() out_dir = Path(out_path).resolve().parent @@ -438,7 +444,10 @@ def make_bundle_source( ).as_posix() except ValueError: overlay_rel = Path(overlay_path).name - return {"contract": contract_rel, "env": env or None, "overlay": overlay_rel} + source: Dict[str, Any] = {"contract": contract_rel, "env": env or None, "overlay": overlay_rel} + if overlay_warnings: + source["overlayWarnings"] = list(overlay_warnings) + return source def read_bundle_source(tgz_path: Path) -> Optional[Dict[str, Any]]: @@ -457,6 +466,20 @@ def read_bundle_source(tgz_path: Path) -> Optional[Dict[str, Any]]: return dict(source) if isinstance(source, dict) else None +def bundle_overlay_warnings(source: Optional[Mapping[str, Any]]) -> List[str]: + """The overlay merge warnings a MANIFEST ``source`` block records. + + ``[]`` when it records none: a bundle built without ``--env``, one whose + overlay merged cleanly, and one built before the record existed. Like the + rest of the block it is outside the merkle root, a hint rather than bundle + content, so only the entries that are non-empty strings are read. + """ + recorded = source.get("overlayWarnings") if source else None + if not isinstance(recorded, list): + return [] + return [warning for warning in recorded if isinstance(warning, str) and warning] + + #: An environment name as a MANIFEST may record it: a plain name, never a #: path. It selects the overlay file whose ``id`` the recorded source #: contract is checked with, so a value like ``../../x`` must not be able to @@ -672,6 +695,7 @@ def _slug(raw: str) -> str: "MANIFEST_VERSION", "build_bundle_tgz", "build_manifest", + "bundle_overlay_warnings", "bundle_source_contract", "extract_fragments", "make_bundle_source", diff --git a/tests/test_overlay_keyed_merge.py b/tests/test_overlay_keyed_merge.py index c8d823b9..27d5b8ce 100644 --- a/tests/test_overlay_keyed_merge.py +++ b/tests/test_overlay_keyed_merge.py @@ -31,6 +31,7 @@ import re import subprocess import sys +import tarfile from datetime import datetime from pathlib import Path from typing import Any, Dict @@ -519,6 +520,161 @@ def test_an_added_build_does_not_fail_strict(self, tmp_path: Path) -> None: assert result.returncode == 0, result.stdout + result.stderr +# A bundle is frozen: `fluid validate .tgz` never merges the overlay +# again, so it cannot see a patch the merge retargeted. `fluid bundle --env` +# records the warning in the bundle's MANIFEST and validate reads it from +# there. Pipeline stage 2 runs exactly `fluid validate --env +# --strict`. + + +def _fluid(cwd: Path, *argv: str) -> "subprocess.CompletedProcess[str]": + return subprocess.run( + [sys.executable, "-m", "fluid_build.cli", *argv], + cwd=str(cwd), + capture_output=True, + text=True, + env=_cli_env(), + ) + + +def _bundle(tmp_path: Path) -> "subprocess.CompletedProcess[str]": + argv = ["contract.fluid.yaml", "--env", "prod", "--format", "tgz", "--out", "b.tgz"] + built = _fluid(tmp_path, "bundle", *argv) + assert built.returncode == 0, built.stdout + built.stderr + return built + + +def _validate_bundle(tmp_path: Path, *argv: str) -> "subprocess.CompletedProcess[str]": + return _fluid(tmp_path, "validate", "b.tgz", "--env", "prod", "--offline", *argv) + + +def _manifest_source(tgz: Path) -> Dict[str, Any]: + with tarfile.open(tgz, "r:gz") as tar: + member = tar.extractfile("MANIFEST.json") + assert member is not None + return json.loads(member.read().decode("utf-8"))["source"] + + +@pytest.fixture(scope="module") +def retargeted_bundle(tmp_path_factory: pytest.TempPathFactory) -> Path: + """A bundle ``fluid bundle --env prod`` built from a positional 0.7.5 overlay.""" + path = tmp_path_factory.mktemp("retargeted") + _write_product(path, "0.7.5", PROD_BY_POSITION) + _bundle(path) + return path + + +class TestValidateBundleStrict: + def test_the_bundle_records_it_and_says_it_once(self, tmp_path: Path) -> None: + _write_product(tmp_path, "0.7.5", PROD_BY_POSITION) + built = _bundle(tmp_path) + out = built.stdout + built.stderr + # Said by the bundle command, not also logged by the loader. + assert out.count("overlay_patch_retargeted") == 1 + assert "overlay warnings: 1 recorded in the bundle" in out + recorded = _manifest_source(tmp_path / "b.tgz")["overlayWarnings"] + assert [w.split(":")[0] for w in recorded] == ["overlay_patch_retargeted"] + assert "the overlay's builds[1] names id transform but patches the base's builds[1], " in ( + recorded[0] + ) + + def test_the_retargeted_patch_is_a_warning(self, retargeted_bundle: Path) -> None: + result = _validate_bundle(retargeted_bundle) + assert result.returncode == 0, result.stdout + result.stderr + # Listed although the bundle passes, as a contract file lists it. + assert ( + "[warning] overlay: MANIFEST.json: overlay_patch_retargeted: in " + "network.service-instances, the overlay's builds[1] names id transform" + ) in " ".join(result.stdout.split()) + + def test_strict_fails_on_it_and_says_why(self, retargeted_bundle: Path) -> None: + result = _validate_bundle(retargeted_bundle, "--strict") + assert result.returncode == 1, result.stdout + result.stderr + assert "overlay_patch_retargeted: in network.service-instances" in result.stdout + assert "Validation failed: warnings treated as errors (--strict)" in result.stdout + + def test_quiet_still_shows_it(self, retargeted_bundle: Path) -> None: + result = _validate_bundle(retargeted_bundle, "--quiet") + assert result.returncode == 0, result.stdout + result.stderr + assert result.stdout.startswith( + "WARNING: overlay: MANIFEST.json: overlay_patch_retargeted: in " + ) + assert (result.stdout + result.stderr).count("overlay_patch_retargeted") == 1 + + def test_quiet_strict_names_it_as_the_error(self, retargeted_bundle: Path) -> None: + result = _validate_bundle(retargeted_bundle, "--quiet", "--strict") + assert result.returncode == 1 + assert result.stdout.startswith( + "ERROR (--strict): overlay: MANIFEST.json: overlay_patch_retargeted: in " + ) + assert (result.stdout + result.stderr).count("overlay_patch_retargeted") == 1 + + def test_strict_json_lists_it(self, retargeted_bundle: Path) -> None: + result = _validate_bundle(retargeted_bundle, "--strict", "--format", "json") + assert result.returncode == 1 + report = json.loads(result.stdout) + assert report["status"] == "fail" + assert [ + (i["severity"], i["file"], i["code"], i["message"].split(":")[0]) + for i in report["issues"] + ] == [("warning", "MANIFEST.json", "OVERLAY-WARNING", "overlay_patch_retargeted")] + + def test_quiet_strict_names_it_beside_other_errors(self, tmp_path: Path) -> None: + # --quiet printed nothing for a bundle, so --strict named the warning + # as the error and hid the real one. + _write_product(tmp_path, "0.7.5", PROD_BY_POSITION) + contract = yaml.safe_load((tmp_path / "contract.fluid.yaml").read_text("utf-8")) + contract["bogusTopLevel"] = 1 + (tmp_path / "contract.fluid.yaml").write_text(yaml.safe_dump(contract), "utf-8") + _bundle(tmp_path) + result = _validate_bundle(tmp_path, "--quiet", "--strict") + assert result.returncode == 1 + lines = result.stdout.splitlines() + assert any( + line.startswith("ERROR: json-schema: contract.resolved.yaml: ") + and "bogusTopLevel" in line + for line in lines + ), result.stdout + assert any( + line.startswith("ERROR (--strict): overlay: MANIFEST.json: overlay_patch_retargeted") + for line in lines + ), result.stdout + + def test_a_keyed_merge_records_nothing(self, tmp_path: Path) -> None: + _write_product(tmp_path, "0.7.6", {"builds": [{"id": "transform", "description": "8"}]}) + _bundle(tmp_path) + assert "overlayWarnings" not in _manifest_source(tmp_path / "b.tgz") + result = _validate_bundle(tmp_path, "--strict") + assert result.returncode == 0, result.stdout + result.stderr + + def test_a_bundle_that_records_none_still_validates(self, tmp_path: Path) -> None: + # Bundles built before the record existed carry no overlayWarnings: + # none recorded, whatever the overlay did. + from fluid_build.cli.bundle import bundle_contract_id, materialize_contract + from fluid_build.forge.core.bundle import build_bundle_tgz, make_bundle_source + + _write_product(tmp_path, "0.7.5", PROD_BY_POSITION) + contract = tmp_path / "contract.fluid.yaml" + compiled, overlay_path = materialize_contract(contract, "prod", logging.getLogger("t")) + out = tmp_path / "b.tgz" + source = make_bundle_source(contract, out, env="prod", overlay_path=overlay_path) + build_bundle_tgz(compiled, out, contract_id=bundle_contract_id(compiled), source=source) + assert "overlayWarnings" not in _manifest_source(out) + result = _validate_bundle(tmp_path, "--strict") + assert result.returncode == 0, result.stdout + result.stderr + + +def test_a_bundle_reads_back_only_the_warnings_it_can_print() -> None: + # The source block is outside the merkle root: a hint, read defensively. + from fluid_build.forge.core.bundle import bundle_overlay_warnings + + assert bundle_overlay_warnings(None) == [] + assert bundle_overlay_warnings({"env": "prod"}) == [] + assert bundle_overlay_warnings({"overlayWarnings": "overlay_patch_retargeted: x"}) == [] + recorded = {"overlayWarnings": ["overlay_patch_retargeted: a", 1, "", None, {"m": 1}, "b"]} + assert bundle_overlay_warnings(recorded) == ["overlay_patch_retargeted: a", "b"] + + def test_workspace_validate_strict_fails_on_it(tmp_path: Path) -> None: # `fluid validate --env prod --strict` with no contract validates every # product in the workspace. From c6615412241eb6da3c7f8a1e34233909bb09f0f3 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:43:49 +0200 Subject: [PATCH 2/6] fix(loader): a collect_overlay_warnings() block that raises still logs what it took The caller reports the warnings a block took only when the block completes, and the loader logs them at DEBUG only. So when another overlay entry failed the merge after a patch had been retargeted, the retargeted patch was never shown. `fluid bundle --env` printed the merge error alone, where main printed both, and `fluid validate --env` and `fluid contract-validation --env` lost it the same way. The block now logs what it took, as the loader logs it, before the error propagates. --- fluid_build/loader.py | 12 +++++++++++- tests/test_overlay_keyed_merge.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/fluid_build/loader.py b/fluid_build/loader.py index bc086618..d096a954 100644 --- a/fluid_build/loader.py +++ b/fluid_build/loader.py @@ -452,12 +452,18 @@ def collect_overlay_warnings( appended to the yielded list instead, once, and logged at DEBUG only: the caller reports it, so the terminal does not show it twice. A warning the block takes is not marked as said, so a later load outside a block still - logs it. + logs it. A block that raises cannot report what it took, such as a patch + retargeted before another overlay entry failed the merge, so those + warnings are logged here before the error propagates. """ sink: List[OverlayWarning] = [] token = _OVERLAY_WARNING_SINK.set((events, sink)) try: yield sink + except Exception: + for warning in sink: + _log_warning_once(warning.message, {"event": warning.event}) + raise finally: _OVERLAY_WARNING_SINK.reset(token) @@ -474,6 +480,10 @@ def _warn_once(event: str, name: str, index: Optional[int], message: str, *args: collecting[1].append(warning) LOG.debug("%s", text, extra=extra) return + _log_warning_once(text, extra) + + +def _log_warning_once(text: str, extra: Dict[str, Any]) -> None: with _NOTED_OVERLAY_MERGE_WARNINGS_LOCK: if text in _NOTED_OVERLAY_MERGE_WARNINGS: return diff --git a/tests/test_overlay_keyed_merge.py b/tests/test_overlay_keyed_merge.py index 27d5b8ce..7ca2f69a 100644 --- a/tests/test_overlay_keyed_merge.py +++ b/tests/test_overlay_keyed_merge.py @@ -398,6 +398,18 @@ def test_outside_a_block_nothing_is_collected(self) -> None: pass assert _OVERLAY_WARNING_SINK.get() is None + def test_a_block_that_raises_logs_what_it_took(self, caplog: pytest.LogCaptureFixture) -> None: + # Its caller reports the warnings only when the block completes: a + # patch retargeted before another entry failed the merge was lost. + from fluid_build.loader import collect_overlay_warnings + + caplog.set_level(logging.WARNING, logger="fluid.loader") + with pytest.raises(OverlayMergeError): + with collect_overlay_warnings(): + _merge(_base("0.7.5"), PROD) + raise OverlayMergeError("the next list failed") + assert _events(caplog) == ["overlay_patch_retargeted"] + def _write_product(tmp_path: Path, version: str, overlay: Dict[str, Any]) -> None: contract = { @@ -647,6 +659,25 @@ def test_a_keyed_merge_records_nothing(self, tmp_path: Path) -> None: result = _validate_bundle(tmp_path, "--strict") assert result.returncode == 0, result.stdout + result.stderr + def test_a_merge_that_then_fails_still_names_the_retargeted_patch(self, tmp_path: Path) -> None: + # Two defects in one overlay: a patch retargeted in builds (whose ids + # repeat, so they merge by position), then an exposes entry with no id. + _write_product(tmp_path, "0.7.6", {}) + contract = yaml.safe_load((tmp_path / "contract.fluid.yaml").read_text("utf-8")) + contract["builds"][0]["id"] = "ingest" + second = copy.deepcopy(contract["exposes"][0]) + second["exposeId"] = "service_instances_v2" + contract["exposes"].append(second) + (tmp_path / "contract.fluid.yaml").write_text(yaml.safe_dump(contract), "utf-8") + overlay = dict(PROD_BY_POSITION, exposes=[{"description": "no exposeId"}]) + (tmp_path / "overlays" / "prod.yaml").write_text(yaml.safe_dump(overlay), "utf-8") + argv = ["contract.fluid.yaml", "--env", "prod", "--format", "tgz", "--out", "b.tgz"] + built = _fluid(tmp_path, "bundle", *argv) + out = built.stdout + built.stderr + assert built.returncode == 1, out + assert "exposes[0] names no exposeId" in out + assert out.count("overlay_patch_retargeted") == 1 + def test_a_bundle_that_records_none_still_validates(self, tmp_path: Path) -> None: # Bundles built before the record existed carry no overlayWarnings: # none recorded, whatever the overlay did. From 8270a0767e0aec0eb570560211d73d173bd0b7e2 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:43:54 +0200 Subject: [PATCH 3/6] fix(validate): a hand-edited overlayWarnings record cannot drive the terminal The record is outside the merkle root, so editing it keeps the bundle digest. An entry carrying escape sequences could move the cursor, erase or forge lines, or write the clipboard in a permissive terminal. A lone surrogate made validate crash before it wrote --report. Each character that is not printable is now escaped as repr writes it, as the block's other values are quoted with !r. _cprint_literal also passes highlight=False, as _cprint_json does. Rich's highlighter takes time quadratic in a run of `<` (4 s for 10,000), and the record put such a run on a passing bundle's output. --- fluid_build/cli/validate.py | 6 ++++-- fluid_build/forge/core/bundle.py | 12 ++++++++++-- tests/test_overlay_keyed_merge.py | 27 +++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/fluid_build/cli/validate.py b/fluid_build/cli/validate.py index 047d6359..6d913712 100644 --- a/fluid_build/cli/validate.py +++ b/fluid_build/cli/validate.py @@ -1206,9 +1206,11 @@ def _cprint_literal(line: str) -> None: Messages are data, not markup, so ``markup=False``, as in :func:`_cprint_json`. The redaction pass in ``cprint`` still runs. ``soft_wrap`` leaves wrapping to the terminal, so Rich never breaks a line - inside an identifier or a copy-pasteable command. + inside an identifier or a copy-pasteable command. ``highlight=False``, as + in :func:`_cprint_json`: Rich's highlighter takes time quadratic in a run + of ``<`` (4 s for 10,000), and a bundle's MANIFEST can carry one. """ - cprint(line, markup=False, soft_wrap=True) + cprint(line, markup=False, soft_wrap=True, highlight=False) def _output_json_results(result: ValidationResult, args) -> int: diff --git a/fluid_build/forge/core/bundle.py b/fluid_build/forge/core/bundle.py index d7b2944c..8ff4ec45 100644 --- a/fluid_build/forge/core/bundle.py +++ b/fluid_build/forge/core/bundle.py @@ -472,12 +472,20 @@ def bundle_overlay_warnings(source: Optional[Mapping[str, Any]]) -> List[str]: ``[]`` when it records none: a bundle built without ``--env``, one whose overlay merged cleanly, and one built before the record existed. Like the rest of the block it is outside the merkle root, a hint rather than bundle - content, so only the entries that are non-empty strings are read. + content: editing it keeps the digest. So only the entries that are + non-empty strings are read, and in each a character that is not printable + is escaped as ``repr`` writes it (``\\x1b``, ``\\n``, ``\\ud800``), as the + block's other values are quoted with ``!r``: printed, an edited entry + cannot move the terminal's cursor, forge a line or fail to encode. """ recorded = source.get("overlayWarnings") if source else None if not isinstance(recorded, list): return [] - return [warning for warning in recorded if isinstance(warning, str) and warning] + return [ + "".join(ch if ch.isprintable() else repr(ch)[1:-1] for ch in warning) + for warning in recorded + if isinstance(warning, str) and warning + ] #: An environment name as a MANIFEST may record it: a plain name, never a diff --git a/tests/test_overlay_keyed_merge.py b/tests/test_overlay_keyed_merge.py index 7ca2f69a..cea1b9d8 100644 --- a/tests/test_overlay_keyed_merge.py +++ b/tests/test_overlay_keyed_merge.py @@ -678,6 +678,30 @@ def test_a_merge_that_then_fails_still_names_the_retargeted_patch(self, tmp_path assert "exposes[0] names no exposeId" in out assert out.count("overlay_patch_retargeted") == 1 + def test_an_edited_record_cannot_drive_the_terminal(self, tmp_path: Path) -> None: + # The record is outside the merkle root: editing it keeps the digest. + # A lone surrogate made validate crash before writing --report. + from fluid_build.forge.core.bundle import write_tgz + + _write_product(tmp_path, "0.7.5", PROD_BY_POSITION) + _bundle(tmp_path) + files: Dict[str, bytes] = {} + with tarfile.open(tmp_path / "b.tgz", "r:gz") as tar: + for member in tar.getmembers(): + handle = tar.extractfile(member) + if handle is not None: + files[member.name] = handle.read() + manifest = json.loads(files["MANIFEST.json"]) + manifest["source"]["overlayWarnings"] = ["x\x1b[1A\x1b[2K\n✅ forged \ud800"] + files["MANIFEST.json"] = json.dumps(manifest).encode("ascii") + write_tgz(tmp_path / "b.tgz", files) + result = _validate_bundle(tmp_path, "--report", "r.json") + assert result.returncode == 0, result.stdout + result.stderr + assert "\x1b" not in result.stdout + assert "x\\x1b[1A\\x1b[2K\\n✅ forged \\ud800" in result.stdout + issues = json.loads((tmp_path / "r.json").read_text("utf-8"))["issues"] + assert [i["code"] for i in issues] == ["OVERLAY-WARNING"] + def test_a_bundle_that_records_none_still_validates(self, tmp_path: Path) -> None: # Bundles built before the record existed carry no overlayWarnings: # none recorded, whatever the overlay did. @@ -704,6 +728,9 @@ def test_a_bundle_reads_back_only_the_warnings_it_can_print() -> None: assert bundle_overlay_warnings({"overlayWarnings": "overlay_patch_retargeted: x"}) == [] recorded = {"overlayWarnings": ["overlay_patch_retargeted: a", 1, "", None, {"m": 1}, "b"]} assert bundle_overlay_warnings(recorded) == ["overlay_patch_retargeted: a", "b"] + # Printable text is kept as written; anything else is escaped as repr writes it. + edited = {"overlayWarnings": ["é [/] a\x1b[2Kb\nc\ud800\u202e\t"]} + assert bundle_overlay_warnings(edited) == ["é [/] a\\x1b[2Kb\\nc\\ud800\\u202e\\t"] def test_workspace_validate_strict_fails_on_it(tmp_path: Path) -> None: From 2ae24f79ce458132e59d2683fc423ff07570fb03 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 21:43:54 +0200 Subject: [PATCH 4/6] docs: the bundle records overlay warnings, and validate fails on them under --strict --- AGENTS.md | 2 +- CHANGELOG.md | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 66971f58..9dbdc764 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,7 +69,7 @@ Generated pipelines therefore compute ONE effective mode for stages 6 and 7: `AP ### Environment and anchoring across stages -- `fluid bundle --env ` freezes the overlay-applied contract and records `source: {contract, env, overlay}` in `MANIFEST.json` (outside the merkle root; `contract` is relative to the bundle's directory and is only trusted when, with the recorded env's overlay applied, it declares the bundled contract's `id`, so an overlay may rename the product). Every later stage given `--env` on a bundle refuses a different env with `bundle_env_mismatch` (a bundle built without `--env` is accepted for `--env dev` when no dev overlay exists); `fluid validate` reports it as a `BUNDLE-ENV-MISMATCH` finding. `fluid generate artifacts` takes `--env` too, and on a raw contract materialises it with the same code as `fluid bundle --env`. +- `fluid bundle --env ` freezes the overlay-applied contract and records `source: {contract, env, overlay}` in `MANIFEST.json`, plus `overlayWarnings` when the merge raised a warning `--strict` fails on (outside the merkle root; `contract` is relative to the bundle's directory and is only trusted when, with the recorded env's overlay applied, it declares the bundled contract's `id`, so an overlay may rename the product). Every later stage given `--env` on a bundle refuses a different env with `bundle_env_mismatch` (a bundle built without `--env` is accepted for `--env dev` when no dev overlay exists); `fluid validate` reports it as a `BUNDLE-ENV-MISMATCH` finding, and each recorded overlay warning as an `OVERLAY-WARNING` one. `fluid generate artifacts` takes `--env` too, and on a raw contract materialises it with the same code as `fluid bundle --env`. - A relative `exposes[].binding.location.path` resolves against the SOURCE contract's directory in every writer and reader (build runners, the local provider, `fluid verify`), whatever directory the command runs from. For a bundle or a bundle-made plan that is the contract the MANIFEST records (`plan.json` carries it as `contract_metadata.source_contract`). - `--env` naming no overlay logs a WARNING (`overlay_not_found`, listing the overlays that exist); `dev` with none is the base contract (INFO). - `fluid apply --state-backend` defaults from `FLUID_STATE_BACKEND`; an explicit empty `--state-backend ""` forces local state. A `FLUID_STATE_BACKEND` that names only a bucket (`s3://bucket`, `gcs://bucket`) keys every contract's state apart (S3 key `fluid//terraform.tfstate`, GCS prefix `fluid/`, the id exactly as written), packaging block or not, because one CI job applies every product with it. Not `safe_ident(id)`: it folds `.` and `-` into `_`, so distinct valid ids would share a state; an id outside the FLUID identifier grammar is refused (`apply_state_backend_invalid`). The flag keeps its old default: `fluid//terraform.tfstate` for a contract with a `packaging` block, the shared `fluid/terraform.tfstate` otherwise. A key or prefix in either one is used as given. The apply output's `state:` line prints the resolved object (`remote: s3://bucket/key (from FLUID_STATE_BACKEND)`), and given back as `--state-backend` that value selects the same state. A bucket name outside `[A-Za-z0-9._-]` is refused without being echoed. Generated pipelines never pass `--state-backend` (a test holds them to it), so the variable's per-contract keys apply. diff --git a/CHANGELOG.md b/CHANGELOG.md index 464df8ad..826e5085 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,12 +37,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 as an "Environment overlay" row, `fluid validate --quiet` as a `WARNING:` line), and the loader no longer also logs it there. Put the overlay's entries in the base's order, or set `fluidVersion: "0.7.6"` with a unique id on every base entry, so overlays patch by - id. `overlay_entry_added` stays a log line and does not fail `--strict`. `fluid validate - .tgz` does not see the warning: `fluid bundle --env` merged the overlay before, - so validate the contract file with `--env`. + id. `overlay_entry_added` stays a log line and does not fail `--strict`. A bundle is + never re-overlaid, so `fluid bundle --env --format tgz` records the warning in the + bundle's MANIFEST (`source.overlayWarnings`) and says how many it recorded; `fluid + validate .tgz` lists it, and under `--strict`, which stage 2 of the generated + pipelines runs, fails on it. A bundle built before this change records none and passes + as before: rebuild it to have it checked. `fluid contract-validation` and `fluid test` + do not read the record, so on a bundle they miss the warning: run them on the contract + file with `--env`. - `fluid validate --strict` now says `Validation failed: warnings treated as errors (--strict)` when warnings alone fail it, after a summary that reads valid; with `--quiet` it prints each such warning as `ERROR (--strict): `. +- `fluid validate .tgz` now prints what it prints for a contract file: a bundle + that passes lists its warnings, not only their count; `--quiet`, which printed nothing, + prints each error as `ERROR: `, a recorded overlay warning as `WARNING: + `, and under `--strict` each warning as `ERROR (--strict): `; and + `--strict` failing on warnings alone says so, as above. - A product whose every build becomes `continuous` (or runs outside forge-cli) gets no `schedule/` artifacts, so pipeline stage 11 skips `fluid schedule-sync`, and a DAG deployed for it earlier stays in the scheduler: remove that DAG by hand. From 097c47e80f1f07c8a7dd13458819ac2d7983be20 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:12:03 +0200 Subject: [PATCH 5/6] refactor(bundle): drop the load_with_overlay import nothing in the module uses --- fluid_build/cli/bundle.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/fluid_build/cli/bundle.py b/fluid_build/cli/bundle.py index 69f6208b..2b6d7320 100644 --- a/fluid_build/cli/bundle.py +++ b/fluid_build/cli/bundle.py @@ -42,12 +42,7 @@ except Exception: # pragma: no cover yaml = None -from ..loader import ( - RefResolutionError, - collect_overlay_warnings, - compile_contract, - load_with_overlay, -) +from ..loader import RefResolutionError, collect_overlay_warnings, compile_contract from ..observability.tracing import traced_stage COMMAND = "bundle" From bdc1c8c659739c556dc1e1d1d9ec7d8bf9a36c9a Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:12:03 +0200 Subject: [PATCH 6/6] test(loader): raise from a helper in the collector error-path test CodeQL read the assertion after `with pytest.raises(...)` as unreachable, because the block's last statement raises. The block now calls a helper that merges and then raises, and the test asserts the error message too. It still checks that a warning taken inside the failing block is logged. --- tests/test_overlay_keyed_merge.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_overlay_keyed_merge.py b/tests/test_overlay_keyed_merge.py index cea1b9d8..a17270bd 100644 --- a/tests/test_overlay_keyed_merge.py +++ b/tests/test_overlay_keyed_merge.py @@ -403,11 +403,14 @@ def test_a_block_that_raises_logs_what_it_took(self, caplog: pytest.LogCaptureFi # patch retargeted before another entry failed the merge was lost. from fluid_build.loader import collect_overlay_warnings - caplog.set_level(logging.WARNING, logger="fluid.loader") - with pytest.raises(OverlayMergeError): + def merge_then_fail() -> None: with collect_overlay_warnings(): _merge(_base("0.7.5"), PROD) raise OverlayMergeError("the next list failed") + + caplog.set_level(logging.WARNING, logger="fluid.loader") + with pytest.raises(OverlayMergeError, match="the next list failed"): + merge_then_fail() assert _events(caplog) == ["overlay_patch_retargeted"]