From 23de60cd23c3f7ccd669cabe9316cb66fd1ac086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Budai?= Date: Fri, 20 Oct 2023 14:26:53 +0200 Subject: [PATCH] stages/mkdir: fix its schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The items of an array are defined under the `items` key, not under `paths`. Let's fix this. Btw, this is possible because JSON Schema itself doesn't use additionalProperties = false. This allows extending the schemas easily, but is sadly a bit error-prone. Sadly, since this issue effectively disabled validation of the stage options, we also need to relax the schema a bit: We found out that there are manifests in the wild, that use relative paths, instead of absolute ones. Thus, this commit changes the validation regex to allow relative paths. However, this now emits a warning and it's strongly discouraged. The associated stage test was modified to accommodate for this. Co-authored-by: Tomáš Hozza Signed-off-by: Tomáš Hozza --- stages/org.osbuild.mkdir | 16 ++++++++++++++-- .../data/manifests/fedora-ostree-bootiso-xz.json | 2 +- .../manifests/fedora-ostree-bootiso-xz.mpp.yaml | 2 +- test/data/manifests/fedora-ostree-bootiso.json | 2 +- .../manifests/fedora-ostree-bootiso.mpp.yaml | 2 +- test/data/stages/mkdir/a.json | 4 ++-- test/data/stages/mkdir/a.mpp.yaml | 4 ++-- test/data/stages/mkdir/b.json | 13 ++++++++----- test/data/stages/mkdir/b.mpp.yaml | 11 ++++++----- test/data/stages/mkdir/diff.json | 3 ++- 10 files changed, 38 insertions(+), 21 deletions(-) diff --git a/stages/org.osbuild.mkdir b/stages/org.osbuild.mkdir index bd234948..1e985ee3 100755 --- a/stages/org.osbuild.mkdir +++ b/stages/org.osbuild.mkdir @@ -11,6 +11,15 @@ directories. If you want to change the mode of an existing directory, you need to use the `org.osbuild.chmod` stage. Mode is applied only to newly created directories and umask value is taken into account. + +In the initial version of this stage, there was a bug that caused +the stage to accept relative paths. This behaviour is kept for +backward compatibility, thus the following paths are equal: + +/path/to/directory +path/to/directory + +However, using relative paths is strongly discouraged. """ import os @@ -26,14 +35,14 @@ SCHEMA_2 = r""" "paths": { "type": "array", "additionalItems": false, - "paths": { + "items": { "type": "object", "additionalProperties": false, "required": ["path"], "properties": { "path": { "type": "string", - "pattern": "^\\/(?!\\.\\.)((?!\\/\\.\\.\\/).)+$" + "pattern": "^\\/?(?!\\.\\.)((?!\\/\\.\\.\\/).)+$" }, "mode": { "type": "number", @@ -64,6 +73,9 @@ def main(tree, options): parents = item.get("parents", False) exist_ok = item.get("exist_ok", False) + if not path.startswith("/"): + print("WARNING: relative path used, this is discouraged!") + target = os.path.join(tree, path.lstrip("/")) if not in_tree(target, tree): raise ValueError(f"path {path} not in tree") diff --git a/test/data/manifests/fedora-ostree-bootiso-xz.json b/test/data/manifests/fedora-ostree-bootiso-xz.json index ba84a168..23e10abe 100644 --- a/test/data/manifests/fedora-ostree-bootiso-xz.json +++ b/test/data/manifests/fedora-ostree-bootiso-xz.json @@ -1921,7 +1921,7 @@ "options": { "paths": [ { - "path": "LiveOS" + "path": "/LiveOS" } ] } diff --git a/test/data/manifests/fedora-ostree-bootiso-xz.mpp.yaml b/test/data/manifests/fedora-ostree-bootiso-xz.mpp.yaml index 2043808a..7ca4f87c 100644 --- a/test/data/manifests/fedora-ostree-bootiso-xz.mpp.yaml +++ b/test/data/manifests/fedora-ostree-bootiso-xz.mpp.yaml @@ -379,7 +379,7 @@ pipelines: - type: org.osbuild.mkdir options: paths: - - path: LiveOS + - path: /LiveOS - type: org.osbuild.truncate options: filename: LiveOS/rootfs.img diff --git a/test/data/manifests/fedora-ostree-bootiso.json b/test/data/manifests/fedora-ostree-bootiso.json index f6d8e89b..34df7c51 100644 --- a/test/data/manifests/fedora-ostree-bootiso.json +++ b/test/data/manifests/fedora-ostree-bootiso.json @@ -1921,7 +1921,7 @@ "options": { "paths": [ { - "path": "LiveOS" + "path": "/LiveOS" } ] } diff --git a/test/data/manifests/fedora-ostree-bootiso.mpp.yaml b/test/data/manifests/fedora-ostree-bootiso.mpp.yaml index 2bf56381..0c6ae912 100644 --- a/test/data/manifests/fedora-ostree-bootiso.mpp.yaml +++ b/test/data/manifests/fedora-ostree-bootiso.mpp.yaml @@ -379,7 +379,7 @@ pipelines: - type: org.osbuild.mkdir options: paths: - - path: LiveOS + - path: /LiveOS - type: org.osbuild.truncate options: filename: LiveOS/rootfs.img diff --git a/test/data/stages/mkdir/a.json b/test/data/stages/mkdir/a.json index 3187a938..d3158e8e 100644 --- a/test/data/stages/mkdir/a.json +++ b/test/data/stages/mkdir/a.json @@ -450,10 +450,10 @@ "options": { "paths": [ { - "path": "a" + "path": "/a" }, { - "path": "c/d", + "path": "/c/d", "parents": true } ] diff --git a/test/data/stages/mkdir/a.mpp.yaml b/test/data/stages/mkdir/a.mpp.yaml index 7476554a..422267a7 100644 --- a/test/data/stages/mkdir/a.mpp.yaml +++ b/test/data/stages/mkdir/a.mpp.yaml @@ -13,6 +13,6 @@ pipelines: - type: org.osbuild.mkdir options: paths: - - path: a - - path: c/d + - path: /a + - path: /c/d parents: true diff --git a/test/data/stages/mkdir/b.json b/test/data/stages/mkdir/b.json index 94fafa27..b2dd4066 100644 --- a/test/data/stages/mkdir/b.json +++ b/test/data/stages/mkdir/b.json @@ -450,24 +450,27 @@ "options": { "paths": [ { - "path": "a" + "path": "/a" }, { - "path": "c/d", + "path": "/c/d", "parents": true }, { - "path": "a/b", + "path": "/a/b", "mode": 448 }, { - "path": "b/c/d", + "path": "/b/c/d", "parents": true }, { - "path": "c", + "path": "/c", "mode": 448, "exist_ok": true + }, + { + "path": "i_am_relative" } ] } diff --git a/test/data/stages/mkdir/b.mpp.yaml b/test/data/stages/mkdir/b.mpp.yaml index 1254a9f0..02308996 100644 --- a/test/data/stages/mkdir/b.mpp.yaml +++ b/test/data/stages/mkdir/b.mpp.yaml @@ -13,13 +13,14 @@ pipelines: - type: org.osbuild.mkdir options: paths: - - path: a - - path: c/d + - path: /a + - path: /c/d parents: true - - path: a/b + - path: /a/b mode: 448 - - path: b/c/d + - path: /b/c/d parents: true - - path: c + - path: /c mode: 448 exist_ok: true + - path: i_am_relative diff --git a/test/data/stages/mkdir/diff.json b/test/data/stages/mkdir/diff.json index 6e2264ff..758c4b1c 100644 --- a/test/data/stages/mkdir/diff.json +++ b/test/data/stages/mkdir/diff.json @@ -3,7 +3,8 @@ "/a/b", "/b", "/b/c", - "/b/c/d" + "/b/c/d", + "/i_am_relative" ], "deleted_files": [], "differences": {}