From ae85001df0043e00372d9854906e45547866c7f4 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 4 Oct 2023 21:02:10 +0200 Subject: [PATCH] Don't leak cwd into MkosiConfig Storing Path.cwd() in MkosiConfig makes it complicated to figure out if a MkosiConfig instance is equal to MkosiConfig.default() as that one executes after changing directory to a temporary directory, so let's remove our default factories for the output and workspace directory and add two methods on the MkosiConfig class instead that replicate the functionality. --- mkosi/__init__.py | 41 ++++++++++++++++++++--------------------- mkosi/config.py | 12 ++++++++---- mkosi/qemu.py | 16 ++++++++-------- 3 files changed, 36 insertions(+), 33 deletions(-) diff --git a/mkosi/__init__.py b/mkosi/__init__.py index 5ae38f1fa..4d160dced 100644 --- a/mkosi/__init__.py +++ b/mkosi/__init__.py @@ -1010,8 +1010,8 @@ def build_initrd(state: MkosiState) -> Path: *(["--compress-output", str(state.config.compress_output)] if state.config.compress_output else []), "--with-network", str(state.config.with_network), "--cache-only", str(state.config.cache_only), - "--output-dir", str(state.config.output_dir), - "--workspace-dir", str(state.config.workspace_dir), + *(["--output-dir", str(state.config.output_dir)] if state.config.output_dir else []), + *(["--workspace-dir", str(state.config.workspace_dir)] if state.config.workspace_dir else []), "--cache-dir", str(state.cache_dir.parent), *(["--local-mirror", str(state.config.local_mirror)] if state.config.local_mirror else []), "--incremental", str(state.config.incremental), @@ -1049,7 +1049,7 @@ def build_initrd(state: MkosiState) -> Path: unlink_output(args, config) build_image(args, config, state.name, state.uid, state.gid) - symlink.symlink_to(config.output_dir / config.output) + symlink.symlink_to(config.output_dir_or_cwd() / config.output) return symlink @@ -1459,8 +1459,8 @@ def unlink_output(args: MkosiArgs, config: MkosiConfig) -> None: prefix = config.output if args.force > 2 else config.output_with_version with complete_step("Removing output files…"): - if config.output_dir.exists(): - for p in config.output_dir.iterdir(): + if config.output_dir_or_cwd().exists(): + for p in config.output_dir_or_cwd().iterdir(): if p.name.startswith(prefix): rmtree(p) @@ -1524,7 +1524,7 @@ def check_outputs(config: MkosiConfig) -> None: config.output_signature if config.sign else None, config.output_nspawn_settings if config.nspawn_settings is not None else None, ): - if f and (config.output_dir / f).exists(): + if f and (config.output_dir_or_cwd() / f).exists(): die(f"Output path {f} exists already. (Consider invocation with --force.)") @@ -1929,7 +1929,7 @@ def finalize_staging(state: MkosiState) -> None: f.rename(state.staging / name) for f in state.staging.iterdir(): - move_tree(state.config, f, state.config.output_dir) + move_tree(state.config, f, state.config.output_dir_or_cwd()) def normalize_mtime(root: Path, mtime: Optional[int], directory: Optional[Path] = None) -> None: @@ -1946,7 +1946,7 @@ def normalize_mtime(root: Path, mtime: Optional[int], directory: Optional[Path] def build_image(args: MkosiArgs, config: MkosiConfig, name: str, uid: int, gid: int) -> None: manifest = Manifest(config) if config.manifest_format else None - workspace = tempfile.TemporaryDirectory(dir=config.workspace_dir, prefix=".mkosi-tmp") + workspace = tempfile.TemporaryDirectory(dir=config.workspace_dir_or_cwd(), prefix=".mkosi-tmp") with workspace, scopedenv({"TMPDIR" : workspace.name}): state = MkosiState(args, config, Path(workspace.name), name, uid, gid) @@ -2038,12 +2038,12 @@ def build_image(args: MkosiArgs, config: MkosiConfig, name: str, uid: int, gid: finalize_staging(state) - output_base = state.config.output_dir / state.config.output + output_base = state.config.output_dir_or_cwd() / state.config.output if not output_base.exists() or output_base.is_symlink(): output_base.unlink(missing_ok=True) output_base.symlink_to(state.config.output_with_compression) - print_output_size(config.output_dir / config.output) + print_output_size(config.output_dir_or_cwd() / config.output) def setfacl(root: Path, uid: int, allow: bool) -> None: @@ -2105,7 +2105,7 @@ def acl_toggle_build(config: MkosiConfig, uid: int) -> Iterator[None]: stack.enter_context(acl_maybe_toggle(config, p, uid, always=True)) if config.output_format == OutputFormat.directory: - stack.enter_context(acl_maybe_toggle(config, config.output_dir / config.output, uid, always=True)) + stack.enter_context(acl_maybe_toggle(config, config.output_dir_or_cwd() / config.output, uid, always=True)) yield @@ -2116,7 +2116,7 @@ def acl_toggle_boot(config: MkosiConfig, uid: int) -> Iterator[None]: yield return - with acl_maybe_toggle(config, config.output_dir / config.output, uid, always=False): + with acl_maybe_toggle(config, config.output_dir_or_cwd() / config.output, uid, always=False): yield @@ -2143,9 +2143,9 @@ def run_shell(args: MkosiArgs, config: MkosiConfig) -> None: with contextlib.ExitStack() as stack: if config.ephemeral: - fname = stack.enter_context(copy_ephemeral(config, config.output_dir / config.output)) + fname = stack.enter_context(copy_ephemeral(config, config.output_dir_or_cwd() / config.output)) else: - fname = config.output_dir / config.output + fname = config.output_dir_or_cwd() / config.output if config.output_format == OutputFormat.disk and args.verb == Verb.boot: run(["systemd-repart", @@ -2193,8 +2193,7 @@ def run_serve(config: MkosiConfig) -> None: port = 8081 - if config.output_dir is not None: - os.chdir(config.output_dir) + os.chdir(config.output_dir_or_cwd()) with http.server.HTTPServer(("", port), http.server.SimpleHTTPRequestHandler) as httpd: logging.info(f"Serving HTTP on port {port}: http://localhost:{port}/") @@ -2300,7 +2299,7 @@ def expand_specifier(s: str) -> str: def needs_build(args: MkosiArgs, config: MkosiConfig) -> bool: return ( args.verb.needs_build() and - (args.force > 0 or not (config.output_dir / config.output_with_compression).exists()) + (args.force > 0 or not (config.output_dir_or_cwd() / config.output_with_compression).exists()) ) @@ -2348,8 +2347,8 @@ def finalize_tools(args: MkosiArgs, presets: Sequence[MkosiConfig]) -> Sequence[ *(["--mirror", p.mirror] if p.mirror and p.distribution == distribution else []), "--repository-key-check", str(p.repository_key_check), "--cache-only", str(p.cache_only), - "--output-dir", str(p.output_dir), - "--workspace-dir", str(p.workspace_dir), + *(["--output-dir", str(p.output_dir)] if p.output_dir else []), + *(["--workspace-dir", str(p.workspace_dir)] if p.workspace_dir else []), *(["--cache-dir", str(p.cache_dir.parent)] if p.cache_dir else []), "--incremental", str(p.incremental), "--acl", str(p.acl), @@ -2374,7 +2373,7 @@ def finalize_tools(args: MkosiArgs, presets: Sequence[MkosiConfig]) -> Sequence[ if config not in new: new.append(config) - new.append(dataclasses.replace(p, tools_tree=config.output_dir / config.output)) + new.append(dataclasses.replace(p, tools_tree=config.output_dir_or_cwd() / config.output)) return new @@ -2506,7 +2505,7 @@ def run_verb(args: MkosiArgs, presets: Sequence[MkosiConfig]) -> None: build_image(args, config, name, uid, gid) # Make sure all build outputs that are not directories are owned by the user running mkosi. - for p in config.output_dir.iterdir(): + for p in config.output_dir_or_cwd().iterdir(): if not p.is_dir(): os.chown(p, uid, gid, follow_symlinks=False) diff --git a/mkosi/config.py b/mkosi/config.py index 7f1c9de5c..87e928f96 100644 --- a/mkosi/config.py +++ b/mkosi/config.py @@ -741,8 +741,8 @@ class MkosiConfig: manifest_format: list[ManifestFormat] output: str compress_output: Compression - output_dir: Path - workspace_dir: Path + output_dir: Optional[Path] + workspace_dir: Optional[Path] cache_dir: Optional[Path] build_dir: Optional[Path] image_id: Optional[str] @@ -842,6 +842,12 @@ class MkosiConfig: preset: Optional[str] + def output_dir_or_cwd(self) -> Path: + return self.output_dir or Path.cwd() + + def workspace_dir_or_cwd(self) -> Path: + return self.workspace_dir or Path.cwd() + @classmethod def default(cls) -> "MkosiConfig": """Alternative constructor to generate an all-default MkosiArgs. @@ -1169,7 +1175,6 @@ SETTINGS = ( specifier="O", parse=config_make_path_parser(required=False), paths=("mkosi.output",), - default_factory=lambda _: Path.cwd(), help="Output directory", ), MkosiConfigSetting( @@ -1179,7 +1184,6 @@ SETTINGS = ( section="Output", parse=config_make_path_parser(required=False), paths=("mkosi.workspace",), - default_factory=lambda _: Path.cwd(), help="Workspace directory", ), MkosiConfigSetting( diff --git a/mkosi/qemu.py b/mkosi/qemu.py index 7f7646d5f..9cb9df574 100644 --- a/mkosi/qemu.py +++ b/mkosi/qemu.py @@ -383,7 +383,7 @@ def run_qemu(args: MkosiArgs, config: MkosiConfig, uid: int, gid: int) -> None: if config.qemu_cdrom and config.output_format == OutputFormat.disk: # CD-ROM devices have sector size 2048 so we transform the disk image into one with sector size 2048. - src = (config.output_dir / config.output).resolve() + src = (config.output_dir_or_cwd() / config.output).resolve() fname = src.parent / f"{src.name}-{uuid.uuid4().hex}" run(["systemd-repart", "--definitions", "", @@ -397,9 +397,9 @@ def run_qemu(args: MkosiArgs, config: MkosiConfig, uid: int, gid: int) -> None: fname]) stack.callback(lambda: fname.unlink()) elif config.ephemeral: - fname = stack.enter_context(copy_ephemeral(config, config.output_dir / config.output)) + fname = stack.enter_context(copy_ephemeral(config, config.output_dir_or_cwd() / config.output)) else: - fname = config.output_dir / config.output + fname = config.output_dir_or_cwd() / config.output if config.output_format == OutputFormat.disk and config.runtime_size: run(["systemd-repart", @@ -415,14 +415,14 @@ def run_qemu(args: MkosiArgs, config: MkosiConfig, uid: int, gid: int) -> None: config.output_format in (OutputFormat.cpio, OutputFormat.uki, OutputFormat.directory) ): if config.output_format == OutputFormat.uki: - kernel = fname if firmware == QemuFirmware.uefi else config.output_dir / config.output_split_kernel + kernel = fname if firmware == QemuFirmware.uefi else config.output_dir_or_cwd() / config.output_split_kernel elif config.qemu_kernel: kernel = config.qemu_kernel elif "-kernel" not in args.cmdline: if firmware == QemuFirmware.uefi: - kernel = config.output_dir / config.output_split_uki + kernel = config.output_dir_or_cwd() / config.output_split_uki else: - kernel = config.output_dir / config.output_split_kernel + kernel = config.output_dir_or_cwd() / config.output_split_kernel if not kernel.exists(): die( f"Kernel or UKI not found at {kernel}, please install a kernel in the image " @@ -458,9 +458,9 @@ def run_qemu(args: MkosiArgs, config: MkosiConfig, uid: int, gid: int) -> None: elif ( firmware == QemuFirmware.linux and config.output_format in (OutputFormat.uki, OutputFormat.directory, OutputFormat.disk) and - (config.output_dir / config.output_split_initrd).exists() + (config.output_dir_or_cwd() / config.output_split_initrd).exists() ): - cmdline += ["-initrd", config.output_dir / config.output_split_initrd] + cmdline += ["-initrd", config.output_dir_or_cwd() / config.output_split_initrd] if config.output_format == OutputFormat.disk: cmdline += ["-drive", f"if=none,id=mkosi,file={fname},format=raw", -- 2.47.3