feat!: Update output path writing

- Allow pull requests to create output paths

Previously, it was not possible to get unmerged pull request outputs. I
would like to access these (e.g. for hosting a preview version of a
website in a similar way to https://docs.netlify.com/site-deploys/deploy-previews/
or https://vercel.com/docs/deployments/preview-deployments)

These are now surfaced under `{owner}/{repo}/pulls/{pr number}`

- Fix repository attribute name conflicts

Previously there was no difference between samely-named attributes in
different repositories.

This has been changed so outputs are under `{owner}/{repo}/{branch}`

- Allow full attribute names, still stripping path traversal

Previously, presumably to prevent path traversal, if your attribute name
contained slashes buildbot-nix would only take the last segment as an
output.

This has been replaced by interpreting slashes as subdirectories and
refusing any segments which don't descend into a subdirectory (i.e. are
attempting path traversal)

- Still create outputs on skipped builds

Previously when something was skipped, for example a build that was
completed in a pull request, the output wouldn't be updated. This made
the outputs directory quite unreliable.

Outputs will now always be updated, no matter whether a build was
actually executed.

BREAKING-CHANGE: This stops old output locations being outputted. If you rely on these locations, you will need to update whatever relies on them.

fix: Skip writing path with failed builds

Previously, if a build failed to produce any output we would try to
write "None" to the output file. This doesn't work, causing the job to
error. Instead, we should skip writing the output and preserve the
original "failed" status.

Author: Minion3665

buildbot_nix/__init__.py

@@ -756,16 +756,57 @@ def __init__(self, project: GitProject, path: Path, **kwargs: Any) -> None:
         self.project = project
         self.path = path
 
+    def join_traversalsafe(self, root: Path, joined: Path) -> Path:
+        root = root.resolve()
+
+        for part in joined.parts:
+            new_root = (root / part).resolve()
+
+            if not new_root.is_relative_to(root):
+                msg = f"Joined path attempted to traverse upwards when processing {root} against {part} (gave {new_root})"
+                raise ValueError(msg)
+
+            root = new_root
+
+        return root
+
+    def join_all_traversalsafe(self, root: Path, *paths: Path) -> Path:
+        for path in paths:
+            root = self.join_traversalsafe(root, path)
+
+        return root
+
+    @defer.inlineCallbacks
     def run(self) -> Generator[Any, object, Any]:
         props = self.build.getProperties()
-        if props.getProperty("branch") != self.project.default_branch:
+
+        pr = props.getProperty("pr_number")
+
+        if not pr and props.getProperty("branch") != self.project.default_branch:
             return util.SKIPPED
 
-        attr = Path(props.getProperty("attr")).name
         out_path = props.getProperty("out_path")
-        # XXX don't hardcode this
-        self.path.mkdir(parents=True, exist_ok=True)
-        (self.path / attr).write_text(out_path)
+
+        if not out_path:  # if, e.g., the build fails and doesn't produce an output
+            return util.SKIPPED
+
+        project_name = Path(props.getProperty("projectname"))
+
+        target = Path(f"pulls/{pr}") if pr else Path(props.getProperty("branch"))
+
+        attr = Path(props.getProperty("attr"))
+
+        try:
+            file = self.join_all_traversalsafe(self.path, project_name, target, attr)
+        except ValueError as e:
+            error_log: StreamLog = yield self.addLog("path_error")
+            error_log.addStderr(f"Path traversal prevented ... skipping update: {e}")
+            return util.FAILURE
+
+        file.parent.mkdir(parents=True, exist_ok=True)
+
+        file.write_text(out_path)
+
         return util.SUCCESS
 
 
@@ -1074,6 +1115,7 @@ def nix_cached_failure_config(
 def nix_skipped_build_config(
     project: GitProject,
     worker_names: list[str],
+    outputs_path: Path | None = None,
 ) -> BuilderConfig:
     """Dummy builder that is triggered when a build is skipped."""
     factory = util.BuildFactory()
@@ -1102,6 +1144,14 @@ def nix_skipped_build_config(
             set_properties={"report_status": False},
         ),
     )
+    if outputs_path is not None:
+        factory.addStep(
+            UpdateBuildOutput(
+                project=project,
+                name="Update build output",
+                path=outputs_path,
+            ),
+        )
     return util.BuilderConfig(
         name=f"{project.name}/nix-skipped-build",
         project=project.name,
@@ -1250,7 +1300,7 @@ def config_for_project(
                 outputs_path=outputs_path,
                 post_build_steps=post_build_steps,
             ),
-            nix_skipped_build_config(project, [SKIPPED_BUILDER_NAME]),
+            nix_skipped_build_config(project, [SKIPPED_BUILDER_NAME], outputs_path),
             nix_failed_eval_config(project, [SKIPPED_BUILDER_NAME]),
             nix_dependency_failed_config(project, [SKIPPED_BUILDER_NAME]),
             nix_cached_failure_config(project, [SKIPPED_BUILDER_NAME]),