CFE-4708: Added support for a path-variable in def.json to specify files to ship beyond just extension-matching

Makefile

@@ -20,8 +20,8 @@ install:
 check: venv format lint
 	uv run pytest
 
-export COVERAGE_PROCESS_START = $(PWD)/.coveragerc
-export COVERAGE_FILE = $(PWD)/.coverage
+coverage: export COVERAGE_PROCESS_START = $(PWD)/.coveragerc
+coverage: export COVERAGE_FILE = $(PWD)/.coverage
 coverage:
 	uv run coverage erase
 	uv run coverage run --parallel-mode -m pytest

cfbs/build.py

@@ -286,19 +286,27 @@ def _localize_file_inputs(name, input_data, destination, build_modules):
     If a response is already shipped by another module's own "directory"
     build step (e.g. the project author set one up manually), that step's
     destination is used instead of making a redundant copy.
+
+    Returns the masterfiles-relative destination path of every file that was
+    localized, so callers can make sure those exact paths get synced by the
+    policy update mechanism even if their extension isn't one of the ones
+    normally recognized.
     """
     if not isinstance(input_data, list):
-        return
+        return []
 
     module_dir_name = name[2:] if name.startswith("./") else name
     module_dir_name = os.path.basename(module_dir_name.rstrip("/"))
 
+    localized_paths = []
+
     def _localize(rel_path):
         if not rel_path or not os.path.isfile(rel_path):
             return rel_path
 
         already_shipped = _path_if_already_shipped(rel_path, build_modules, destination)
         if already_shipped is not None:
+            localized_paths.append(strip_left(already_shipped, "$(sys.inputdir)/"))
             return already_shipped
 
         rel_path = os.path.normpath(rel_path)
@@ -311,8 +319,22 @@ def _localize(rel_path):
             "modules" if in_module_dir else "",
             rel_path,
         )
+        abs_destination = os.path.abspath(destination)
+        if (
+            os.path.commonpath([os.path.abspath(dest), abs_destination])
+            != abs_destination
+        ):
+            # rel_path contained a ".." segment, or was absolute (which
+            # discards the destination prefix in os.path.join above) -
+            # either way it would land outside the built masterfiles.
+            raise CFBSExitError(
+                "Input file response '%s' would be placed outside the "
+                "built masterfiles - refusing to copy it" % rel_path
+            )
         cp(rel_path, dest)
-        return "$(sys.inputdir)/" + os.path.relpath(dest, destination)
+        dest_rel = os.path.relpath(dest, destination)
+        localized_paths.append(dest_rel)
+        return "$(sys.inputdir)/" + dest_rel
 
     for element in input_data:
         if not isinstance(element, dict) or element.get("type") != "file":
@@ -323,6 +345,51 @@ def _localize(rel_path):
         else:
             element["response"] = _localize(response)
 
+    return localized_paths
+
+
+MIN_MASTERFILES_VERSION_FOR_INPUT_PATHS_EXTRA = (3, 29)
+
+
+def _warn_if_input_paths_extra_unsupported(destination, build_modules):
+    """input_paths_extra (CFE-4708) is only understood by masterfiles
+    3.29.0+ - on an older target it's just an unused variable in def.json,
+    so the file(s) it names won't actually get synced to clients.
+    """
+    def_json = read_json(os.path.join(destination, "def.json"))
+    if not def_json:
+        return
+    if not def_json.get("vars", {}).get("default:update_def.input_paths_extra"):
+        return
+
+    masterfiles = next(
+        (m for m in build_modules if m.get("name") == "masterfiles"), None
+    )
+    version = masterfiles.get("version") if masterfiles else None
+    if not version:
+        # Not an index-added "masterfiles" module (local copy)
+        # nothing to check the version of.
+        # Assume the user has the latest version of masterfiles
+        return
+
+    parts = version.split(".")
+    try:
+        found = (int(parts[0]), int(parts[1]))
+    except (IndexError, ValueError):
+        return
+    if found >= MIN_MASTERFILES_VERSION_FOR_INPUT_PATHS_EXTRA:
+        return
+
+    log.warning(
+        "'input_paths_extra' was written to def.json, but the target masterfiles version (%s) predates %s, "
+        "which is when it was added - the file(s) dependent on this variable "
+        "(those whose extensions are not defined in input_name_patterns) may silently not get synced to clients."
+        % (
+            version,
+            ".".join(str(n) for n in MIN_MASTERFILES_VERSION_FOR_INPUT_PATHS_EXTRA),
+        )
+    )
+
 
 def _perform_input_step(args, name, destination, prefix, build_modules):
     src, dst = args
@@ -345,14 +412,24 @@ def _perform_input_step(args, name, destination, prefix, build_modules):
         )
         return
     extras, original = read_json(src), read_json(dst)
-    _localize_file_inputs(name, extras, destination, build_modules)
+    localized_paths = _localize_file_inputs(name, extras, destination, build_modules)
     extras = generate_augment(name, extras)
     log.debug("Generated augment: %s", pretty(extras))
     if not extras:
         raise CFBSExitError(
             "Input data '%s' is incomplete: Skipping build step."
             % os.path.basename(src)
         )
+    if localized_paths:
+        # Files brought in through "file" type inputs aren't necessarily
+        # matched by the policy update's default `input_name_patterns`.
+        # Rather than widening that extension-based matching for the whole
+        # policy set, point at exactly these files, by their literal
+        # relative path.
+        relative_paths = [path.replace(os.sep, "/") for path in localized_paths]
+        extras = merge_json(
+            extras, {"vars": {"default:update_def.input_paths_extra": relative_paths}}
+        )
     if original:
         log.debug("Original def.json: %s", pretty(original))
         merged = merge_json(original, extras)
@@ -541,6 +618,7 @@ def perform_build(config: CFBSConfig, diffs_filename=None) -> int:
             raise CFBSExitError(
                 "Error parsing JSON in 'out/masterfiles/def.json': %s" % e
             )
+    _warn_if_input_paths_extra_unsupported("out/masterfiles", config["build"])
     print("")
     print("Generating tarball...")
     sh("( cd out/ && tar -czf masterfiles.tgz masterfiles )")

tests/test_build.py

@@ -1,7 +1,12 @@
 import os
 import copy
+import json
+import tempfile
 
-from cfbs.build import _localize_file_inputs
+import pytest
+
+from cfbs.build import _localize_file_inputs, _perform_input_step
+from cfbs.utils import CFBSExitError
 
 
 def test_localize_file_inputs_copies_single_file(tmp_path, monkeypatch):
@@ -18,11 +23,14 @@ def test_localize_file_inputs_copies_single_file(tmp_path, monkeypatch):
         }
     ]
 
-    _localize_file_inputs("run-a-script", input_data, "out/masterfiles", [])
+    localized_paths = _localize_file_inputs(
+        "run-a-script", input_data, "out/masterfiles", []
+    )
 
     expected_dest = "out/masterfiles/services/cfbs/deploy.sh"
     assert os.path.isfile(expected_dest)
     assert input_data[0]["response"] == "$(sys.inputdir)/services/cfbs/deploy.sh"
+    assert localized_paths == ["services/cfbs/deploy.sh"]
 
 
 def test_localize_file_inputs_copies_list_of_files(tmp_path, monkeypatch):
@@ -42,7 +50,9 @@ def test_localize_file_inputs_copies_list_of_files(tmp_path, monkeypatch):
         }
     ]
 
-    _localize_file_inputs("run-scripts-module", input_data, "out/masterfiles", [])
+    localized_paths = _localize_file_inputs(
+        "run-scripts-module", input_data, "out/masterfiles", []
+    )
 
     assert input_data[0]["response"] == [
         "$(sys.inputdir)/services/cfbs/one.sh",
@@ -52,6 +62,10 @@ def test_localize_file_inputs_copies_list_of_files(tmp_path, monkeypatch):
     assert os.path.isfile(
         "out/masterfiles/services/cfbs/modules/run-scripts-module/two.sh"
     )
+    assert localized_paths == [
+        "services/cfbs/one.sh",
+        "services/cfbs/modules/run-scripts-module/two.sh",
+    ]
 
 
 def test_localize_file_inputs_strips_local_module_prefix_with_dot_slash(
@@ -113,6 +127,33 @@ def test_localize_file_inputs_ignores_missing_file(tmp_path, monkeypatch):
     assert input_data == before
 
 
+def test_localize_file_inputs_rejects_path_traversal(tmp_path, monkeypatch):
+    """An absolute (or '..'-laden) response would otherwise let os.path.join
+    discard the destination prefix, placing the file outside the built
+    masterfiles entirely - refuse it instead of writing there."""
+    monkeypatch.chdir(tmp_path)
+    os.makedirs("out/masterfiles")
+
+    with tempfile.NamedTemporaryFile(suffix=".txt", delete=False) as f:
+        f.write(b"pwned\n")
+        outside_path = f.name
+
+    try:
+        input_data = [{"type": "file", "variable": "script", "response": outside_path}]
+
+        with pytest.raises(CFBSExitError):
+            _localize_file_inputs("some-module", input_data, "out/masterfiles", [])
+
+        assert not os.path.exists(
+            "out/masterfiles/services/cfbs/" + os.path.basename(outside_path)
+        )
+    finally:
+        try:
+            os.unlink(outside_path)
+        except OSError:
+            pass
+
+
 def test_localize_file_inputs_ignores_non_file_types(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
     os.makedirs("out/masterfiles")
@@ -191,3 +232,52 @@ def test_localize_file_inputs_ignores_non_directory_steps(tmp_path, monkeypatch)
         "$(sys.inputdir)/services/cfbs/modules/run-scripts/deploy.sh"
     )
     assert os.path.isfile("out/masterfiles/services/cfbs/modules/run-scripts/deploy.sh")
+
+
+def test_perform_input_step_adds_input_paths_extra_for_localized_files(
+    tmp_path, monkeypatch
+):
+    """A "file" type input should make the build add its exact destination
+    path to `default:update_def.input_paths_extra`, so the policy update
+    mechanism syncs it even if its extension isn't in the default
+    `input_name_patterns` list.
+    """
+    monkeypatch.chdir(tmp_path)
+    os.makedirs("out/masterfiles")
+    os.makedirs("run-a-script")
+    with open("deploy.sh", "w") as f:
+        f.write("echo hi\n")
+    with open("run-a-script/input.json", "w") as f:
+        json.dump([{"type": "file", "variable": "script", "response": "deploy.sh"}], f)
+
+    _perform_input_step(
+        ["./input.json", "def.json"], "run-a-script", "out/masterfiles", "+", []
+    )
+
+    with open("out/masterfiles/def.json") as f:
+        result = json.load(f)
+
+    expected_path = "services/cfbs/deploy.sh"
+    assert result["vars"]["default:update_def.input_paths_extra"] == [expected_path]
+
+
+def test_perform_input_step_skips_input_paths_extra_for_non_file_inputs(
+    tmp_path, monkeypatch
+):
+    """A build with only "string"-type inputs has nothing to localize, so no
+    `input_paths_extra` augment should be generated at all.
+    """
+    monkeypatch.chdir(tmp_path)
+    os.makedirs("out/masterfiles")
+    os.makedirs("some-module")
+    with open("some-module/input.json", "w") as f:
+        json.dump([{"type": "string", "variable": "greeting", "response": "hello"}], f)
+
+    _perform_input_step(
+        ["./input.json", "def.json"], "some-module", "out/masterfiles", "+", []
+    )
+
+    with open("out/masterfiles/def.json") as f:
+        result = json.load(f)
+
+    assert "vars" not in result