Skip to content

make-disk-image: allow pre/post format files in vm #778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

make-disk-image: allow pre/post format files in vm #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 47 additions & 20 deletions lib/make-disk-image.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,26 @@ let
util-linux
findutils
] ++ cfg.extraDependencies;

prepareFile = name: content: ''
out="$(echo "${name}" | base64)"
${if lib.isStorePath content
then ''cp --reflink=auto -r "${content}" "$out"''
else content
}
'';

prepareFiles = ''
(
cd $TMPDIR/xchg
mkdir -p pre_format_files post_format_files
cd pre_format_files
${lib.concatStringsSep "\n" (lib.attrValues (lib.mapAttrs prepareFile cfg.preFormatFiles))}
cd ../post_format_files
${lib.concatStringsSep "\n" (lib.attrValues (lib.mapAttrs prepareFile cfg.postFormatFiles))}
)
'';

preVM = ''
${lib.concatMapStringsSep "\n" (disk: "${pkgs.qemu}/bin/qemu-img create -f ${imageFormat} ${disk.name}.${imageFormat} ${disk.imageSize}") (lib.attrValues diskoCfg.devices.disk)}
# This makes disko work, when canTouchEfiVariables is set to true.
Expand All @@ -59,8 +79,19 @@ let
closureInfo = pkgs.closureInfo {
rootPaths = [ systemToInstall.config.system.build.toplevel ];
};

partitioner = ''
set -efux
set -eux

set +f
for src in /tmp/xchg/pre_format_files/*; do
[ -e "$src" ] || continue
dst=$(basename "$src" | base64 -d)
mkdir -p "$(dirname "$dst")"
cp -r "$src" "$dst"
done
set -f

# running udev, stolen from stage-1.sh
echo "running udev..."
ln -sfn /proc/self/fd /dev/fd
Expand All @@ -80,6 +111,15 @@ let
export IN_DISKO_TEST=1
''}
${systemToInstall.config.system.build.diskoScript}

set +f
for src in /tmp/xchg/post_format_files/*; do
[ -e "$src" ] || continue
dst=/mnt/$(basename "$src" | base64 -d)
mkdir -p "$(dirname "$dst")"
cp -r "$src" "$dst"
done
set -f
'';

installer = lib.optionalString cfg.copyNixStore ''
Expand Down Expand Up @@ -108,8 +148,9 @@ in
system.build.diskoImages = vmTools.runInLinuxVM (pkgs.runCommand cfg.name
{
buildInputs = dependencies;
inherit preVM postVM QEMU_OPTS;
inherit postVM QEMU_OPTS;
inherit (diskoCfg) memSize;
preVM = preVM + prepareFiles;
}
(partitioner + installer));

Expand Down Expand Up @@ -140,20 +181,20 @@ in
trap 'rm -rf "$TMPDIR"' EXIT
cd "$TMPDIR"

mkdir copy_before_disko copy_after_disko
mkdir pre_format_files post_format_files

while [[ $# -gt 0 ]]; do
case "$1" in
--pre-format-files)
src=$2
dst=$3
cp --reflink=auto -r "$src" copy_before_disko/"$(echo "$dst" | base64)"
cp --reflink=auto -r "$src" pre_format_files/"$(echo "$dst" | base64)"
shift 2
;;
--post-format-files)
src=$2
dst=$3
cp --reflink=auto -r "$src" copy_after_disko/"$(echo "$dst" | base64)"
cp --reflink=auto -r "$src" post_format_files/"$(echo "$dst" | base64)"
shift 2
;;
--build-memory)
Expand All @@ -175,25 +216,11 @@ in

export preVM=${diskoLib.writeCheckedBash { inherit pkgs checked; } "preVM.sh" ''
set -efu
mv copy_before_disko copy_after_disko xchg/
mv pre_format_files post_format_files xchg/
origBuilder=${pkgs.writeScript "disko-builder" ''
set -eu
export PATH=${lib.makeBinPath dependencies}
for src in /tmp/xchg/copy_before_disko/*; do
[ -e "$src" ] || continue
dst=$(basename "$src" | base64 -d)
mkdir -p "$(dirname "$dst")"
cp -r "$src" "$dst"
done
set -f
${partitioner}
set +f
for src in /tmp/xchg/copy_after_disko/*; do
[ -e "$src" ] || continue
dst=/mnt/$(basename "$src" | base64 -d)
mkdir -p "$(dirname "$dst")"
cp -r "$src" "$dst"
done
${installer}
''}
echo "export origBuilder=$origBuilder" > xchg/saved-env
Expand Down
41 changes: 41 additions & 0 deletions module.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,47 @@ in
default = { };
};

preFormatFiles = lib.mkOption {
type = lib.types.attrsOf (lib.types.either lib.types.str lib.types.path);
description = ''
Files to copy into the image builder VM before disko is run.
This is useful to provide secrets like LUKS keys, or other files you need for formatting.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not have a disclaimer, that this leaks secrets into the nix store?
For image builder that runs outside of the nix store, we have a parameter that can be used instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A warning that it could leak is probably a good idea.

But the second variant of passing a shell snippet should be usable without leaking secrets into the store? i.e.

preFormatFiles."/tmp/secret" = "cat /my-secret-out-side-nix store > $out" shouldn't leak. $out is just a tempfile here, see prepareFiles above.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"cat /my-secret-outside-nix > $out" doesn't work:

error: builder for '/nix/store/ragb6hgfl9wvdyb9g6721mfyymn3w0mg-myvm-disko-images.drv' failed with exit code 1;
       last 2 log lines:
       > Formatting 'main.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=1073741824 lazy_refcounts=off refcount_bits=16
       > cat: /path/to/secret.txt: No such file or directory

I think this is because the cat command is being executed inside the builder sandbox, so it cannot access files outside the store, or something like that.

Copy link

@musjj musjj Apr 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it not possible to add the files outside of nix's build process (e.g. via a shell script run externally by disko)? I think leaking secrets like this is a huge footgun. Maybe we should have a new command that can perform all these necessary operations outside of nix e.g. disko-vm.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this some more and I think this is a fatal flaw in the design of this PR. Files containing cleartext secrets necessarily introduce an impurity into the process of building the image, so they really need to be brought in the way diskoImagesScript does it, i.e. with a postprocessing step after the Nix builder is done.

Copy link
Member

@trueNAHO trueNAHO Aug 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about encouraging fake dummy secrets instead of real production secrets, since there is little benefit in testing the production secret in the VM compared to some dummy secret:

options.disko.imageBuilder.preFormatFiles = lib.mkOption {
  description = ''
    ${before}

    This is useful for providing secrets, like LUKS keys or other files needed
    for formatting. Since these secrets will be world readable, it is strongly
    encouraged to use dummy secrets.

    ${after}
  '';

  example = lib.literalExpression ''
    {
      "/tmp/secret.key" = builtins.toFile "password-file" "password";
    }
  '';
};

On a side note, it would be nice if empty passphrases would also work to avoid typing any password at all. The following diskoImagesScript usage seems broken for LUKS:

${config.system.build.diskoImagesScript}
  --pre-format-files ${pkgs.emptyFile} /tmp/secret.key


Names are interpreted as destination paths. If the value is a store path,
that path will be copied as-is. If it's not a store path, the value will be interpreted
as shell code is expected to write files into $out.
'';
default = {};
example = lib.literalExpression ''
{
"/tmp/pre/file" = pkgs.writeText "foo" "bar";
"/tmp/pre/script" = "mkdir -p $out/foo; echo bar > $out/foo";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"/tmp/pre/script" = "mkdir -p $out/foo; echo bar > $out/foo";
"/tmp/pre/script" = "mkdir -p $out; echo bar > $out/foo";

}
'';
};

postFormatFiles = lib.mkOption {
type = lib.types.attrsOf (lib.types.either lib.types.str lib.types.path);
description = ''
Files to copy into the final image, after disko has run.
These end up in the images later and is useful if you want to add some extra stateful files
They will have the same permissions but will be owned by root:root.

Names are interpreted as destination paths. If the value is a store path,
that path will be copied as-is. If it's not a store path, the value will be interpreted
as shell code is expected to write files into $out.
'';
default = {};
example = lib.literalExpression ''
{
"/tmp/pre/file" = pkgs.writeText "foo" "bar";
"/tmp/pre/script" = "mkdir -p $out/foo; echo bar > $out/foo";
}
'';
};



imageFormat = lib.mkOption {
type = lib.types.enum [ "raw" "qcow2" ];
description = "QEMU image format to use for the disk images";
Expand Down