From 88d4cf8a0096be590fc7d7172a9e202ce4403b2a Mon Sep 17 00:00:00 2001
From: Will Thompson <wjt@endlessos.org>
Date: Fri, 21 Jun 2024 14:47:50 +0100
Subject: [PATCH 1/2] Add validation for paths in config

Previously, if a filename referred to in the configuration does not
exist, that would only be discovered when the hook which uses that key
runs, which may be minutes or hours into the build.

Add a way to specify in the schema that certain keys must be set to a
single path to an extant file, or a list of paths to extant files.

https://phabricator.endlessm.com/T35517
---
 README.md                   |  4 +++
 config/schema.ini           |  3 +++
 run-build                   | 20 +++++++++++++++
 tests/test_image_builder.py | 49 +++++++++++++++++++++++++++++++++++++
 4 files changed, 76 insertions(+)

diff --git a/README.md b/README.md
index 2cc4f885..eaed52fe 100644
--- a/README.md
+++ b/README.md
@@ -190,6 +190,10 @@ with key suffixes as follows:
  * `_required`: `true` means that the key must be set
  * `_values`: the value, if set, must be within the space-separated list
    of values here
+ * `_type`: `path` means the value, if set, must be the path to a file which
+   exists
+ * `_type`: `paths` means the value, if set, must be a space-separated list of
+   path to files which exist
 
 Merged options
 --------------
diff --git a/config/schema.ini b/config/schema.ini
index 01d4eafa..c5cd7147 100644
--- a/config/schema.ini
+++ b/config/schema.ini
@@ -4,6 +4,9 @@
 #  _required = true means that the key must be set
 #  _values means the value, if set, must be within the space-separated list of
 #          values here
+#  _type = path means the value, if set, must be the path to a file which exists
+#  _type = paths means the value, if set, must be a space-separated list of
+#                path to files which exist
 
 [image]
 compression_required = true
diff --git a/run-build b/run-build
index 4141e522..1e3b495c 100755
--- a/run-build
+++ b/run-build
@@ -515,6 +515,26 @@ class ImageBuilder(object):
                 raise eib.ImageBuildError(
                     'Configuration key [%s] %s has invalid value: %s'
                     % (section, option, self.config[section][option]))
+        elif option.endswith('_type'):
+            real_option = option[:-len('_type')]
+            if real_option in self.config[section]:
+                real_value = self.config[section][real_option].strip()
+                if value == "path":
+                    paths = [real_value] if real_value else []
+                elif value == "paths":
+                    paths = real_value.split()
+                else:
+                    raise eib.ImageBuildError(
+                        f'Schema key [{section}] {option} has invalid value: '
+                        f'{value}'
+                    )
+
+                for path in paths:
+                    if not os.path.exists(path):
+                        raise eib.ImageBuildError(
+                            f'Configuration key [{section}] {real_option} refers to '
+                            f'nonexistent path: {path}'
+                        )
 
     def check_config(self):
         """Check loaded configuration against schema for validity."""
diff --git a/tests/test_image_builder.py b/tests/test_image_builder.py
index b7dd188c..5e943bd2 100644
--- a/tests/test_image_builder.py
+++ b/tests/test_image_builder.py
@@ -327,3 +327,52 @@ def test_localdir(make_builder, tmp_path, tmp_builder_paths, caplog):
     assert (builder.config['image']['branding_desktop_logo'] ==
             str(localdir / 'data' / 'desktop.png'))
     assert builder.config['image']['signing_key'] == 'foobar'
+
+
+def test_path_validation(make_builder, tmp_path):
+    configdir = tmp_path / 'config'
+    configdir.mkdir()
+
+    # Schema
+    schema = configdir / 'schema.ini'
+    schema.write_text(dedent("""\
+    [image]
+    singular_type = path
+    plural_type = paths
+    """))
+
+    # Some config
+    defaults = configdir / 'defaults.ini'
+    defaults.write_text(dedent("""\
+    [image]
+    singular = ${build:localdatadir}/a.txt
+    plural = ${build:localdatadir}/b.txt ${build:localdatadir}/c.txt
+    """))
+
+    localdir = tmp_path / 'local'
+    localdatadir = localdir / 'data'
+    localdatadir.mkdir(parents=True)
+
+    a = localdatadir / 'a.txt'
+    a.touch()
+
+    b = localdatadir / 'b.txt'
+    b.touch()
+
+    c = localdatadir / 'c.txt'
+    c.touch()
+
+    builder = make_builder(configdir=str(configdir), localdir=str(localdir))
+    builder.configure()
+
+    # All paths exist
+    builder.check_config()
+
+    b.unlink()
+    with pytest.raises(eib.ImageBuildError, match=r'plural.*b.txt'):
+        builder.check_config()
+
+    b.touch()
+    a.unlink()
+    with pytest.raises(eib.ImageBuildError, match=r'singular.*a.txt'):
+        builder.check_config()

From 8f8d7206f53492e12e61dd3e228feb91776919d8 Mon Sep 17 00:00:00 2001
From: Will Thompson <wjt@endlessos.org>
Date: Fri, 21 Jun 2024 15:00:12 +0100
Subject: [PATCH 2/2] Specify path(s) types in config schema

Based on a quick review of the hooks in this repo, I think this covers
all the likely candidates.

https://phabricator.endlessm.com/T35517
---
 config/schema.ini | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/config/schema.ini b/config/schema.ini
index c5cd7147..312bdfd8 100644
--- a/config/schema.ini
+++ b/config/schema.ini
@@ -14,3 +14,10 @@ compression_values = gz
                      xz
 partition_table_values = gpt
                          dos
+branding_fbe_config_type = path
+icon_grid_type = paths
+initramfs_plymouth_watermark = path
+chromium_policies_managed = path
+chromium_policies_recommended = path
+settings = paths
+settings_locks = paths