From 70dfe2521154a0e1b25ec93b82f63e559eb53fc4 Mon Sep 17 00:00:00 2001 From: Skyler Grey Date: Sat, 27 Jul 2024 12:48:41 +0000 Subject: [PATCH] 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. --- buildbot_nix/__init__.py | 62 ++++++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/buildbot_nix/__init__.py b/buildbot_nix/__init__.py index 932708b..185e34c 100644 --- a/buildbot_nix/__init__.py +++ b/buildbot_nix/__init__.py @@ -310,18 +310,55 @@ class UpdateBuildOutput(steps.BuildStep): 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) - return util.SUCCESS + owner = Path(props.getProperty("owner")) + repo = Path(props.getProperty("repository_name")) + target = Path(props.getProperty("branch")) if not pr else Path(f"pulls/{pr}") + + attr = Path(props.getProperty("attr")) + + try: + file = self.join_all_traversalsafe(self.path, owner, repo, 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) + + out_path = props.getProperty("out_path") + file.write_text(out_path) + + return util.SUCCESS # GitHub somtimes fires the PR webhook before it has computed the merge commit # This is a workaround to fetch the merge commit and checkout the PR branch in CI @@ -563,6 +600,7 @@ def nix_build_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() @@ -601,6 +639,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, @@ -759,7 +805,7 @@ def config_for_project( retries=build_retries, 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_register_gcroot_config(project, worker_names), nix_build_combined_config(project, worker_names), ], -- 2.45.2