Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 16 additions & 3 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
pi = { suffix = "-pi"; agentModule = ./modules/agents/pi.nix; dataDirName = "pi-microvm"; apiKeyVars = [ "ANTHROPIC_API_KEY" "OPENAI_API_KEY" "GEMINI_API_KEY" ]; };
};

mkRunnerScript = { pkgs, runner, dataDirName, apiKeyVars, agentName }:
mkRunnerScript = { pkgs, runner, dataDirName, apiKeyVars, agentName, defaultMem, defaultVcpu }:
let
virtiofsd = pkgs.virtiofsd;
hostname = "${agentName}-vm";
Expand All @@ -32,6 +32,8 @@
in pkgs.writeShellScriptBin "microvm-run" ''
set -euo pipefail
WORK="$(realpath "''${WORK_DIR:-$(pwd)}")"
VM_MEM="''${VM_MEM:-${toString defaultMem}}"
VM_VCPU="''${VM_VCPU:-${toString defaultVcpu}}"
Comment on lines +35 to +36

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add runtime validation for VM_MEM and VM_VCPU.

The script reads VM_MEM and VM_VCPU from the environment without validation. Invalid values (non-numeric, negative, zero) will be passed to QEMU, potentially causing confusing errors.

🛡️ Proposed validation
 VM_MEM="''${VM_MEM:-${toString defaultMem}}"
 VM_VCPU="''${VM_VCPU:-${toString defaultVcpu}}"
+
+# Validate VM_MEM is a positive integer
+if ! [[ "$VM_MEM" =~ ^[0-9]+$ ]] || [ "$VM_MEM" -le 0 ]; then
+  echo "error: VM_MEM must be a positive integer (got: $VM_MEM)" >&2
+  exit 1
+fi
+
+# Validate VM_VCPU is a positive integer
+if ! [[ "$VM_VCPU" =~ ^[0-9]+$ ]] || [ "$VM_VCPU" -le 0 ]; then
+  echo "error: VM_VCPU must be a positive integer (got: $VM_VCPU)" >&2
+  exit 1
+fi
+
 RUNTIME="''${XDG_RUNTIME_DIR:-/run/user/$(id -u)}"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@flake.nix` around lines 35 - 36, Validate VM_MEM and VM_VCPU at runtime
before passing them to QEMU: check that VM_MEM and VM_VCPU (used in flake.nix
where VM_MEM="''${VM_MEM:-${toString defaultMem}}",
VM_VCPU="''${VM_VCPU:-${toString defaultVcpu}}") are positive integers (numeric,
>0) and fall back to the default values (defaultMem/defaultVcpu) or fail fast
with a clear error if they are invalid; implement the check using a Nix
expression (e.g., builtins.match or parseInt + comparisons) and replace the
direct interpolations with the validated/normalized variables so non-numeric,
zero or negative inputs are rejected or replaced before being handed to QEMU.

RUNTIME="''${XDG_RUNTIME_DIR:-/run/user/$(id -u)}"
ID="$(cat /proc/sys/kernel/random/uuid)"

Expand Down Expand Up @@ -256,6 +258,10 @@
-e "s|${hostname}-virtiofs-agent-home.sock|$AGENT_SOCK|g"
-e "s|/tmp/${hostname}-cri-storage|$CRI_DIR|g"
-e "s|${hostname}-virtiofs-cri-storage.sock|$CRI_SOCK|g"
# Runtime mem/vcpu override (VM_MEM / VM_VCPU env vars)
-e "s| -m ${toString defaultMem} | -m $VM_MEM |g"
-e "s| -smp ${toString defaultVcpu} | -smp $VM_VCPU |g"
-e "s|size=${toString defaultMem}M|size=''${VM_MEM}M|g"
)

# Run QEMU with corrected paths
Expand Down Expand Up @@ -321,8 +327,15 @@
packages = forSystems (system: let pkgs = nixpkgs.legacyPackages.${system}; in
{ default = self.packages.${system}.claude; } //
builtins.mapAttrs (name: flavor: let
runner = self.nixosConfigurations."${name}${flavor.suffix}-${system}".config.microvm.runner.qemu;
in mkRunnerScript { inherit pkgs runner; inherit (flavor) dataDirName apiKeyVars; agentName = name; }) vmFlavors
nixosCfg = self.nixosConfigurations."${name}${flavor.suffix}-${system}".config;
runner = nixosCfg.microvm.runner.qemu;
in mkRunnerScript {
inherit pkgs runner;
inherit (flavor) dataDirName apiKeyVars;
agentName = name;
defaultMem = nixosCfg.claude-vm.agent.mem;
defaultVcpu = nixosCfg.claude-vm.agent.vcpu;
}) vmFlavors
);
};
}
14 changes: 12 additions & 2 deletions modules/base.nix
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ in
default = "";
description = "Agent-specific shell init (runs before launch)";
};
mem = lib.mkOption {
type = lib.types.int;
default = 8192;
description = "VM memory in MB. Overridable at runtime via VM_MEM env var.";
};
vcpu = lib.mkOption {
type = lib.types.int;
default = 4;
description = "VM vCPU count. Overridable at runtime via VM_VCPU env var.";
};
Comment on lines +25 to +34

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add validation to ensure positive values.

The mem and vcpu options accept any integer, including negative or zero values. While QEMU would reject these at runtime, adding Nix-level validation provides clearer error messages during configuration.

✅ Proposed validation
     mem = lib.mkOption {
       type = lib.types.int;
       default = 8192;
       description = "VM memory in MB. Overridable at runtime via VM_MEM env var.";
     };
+    # Validate mem is positive
+    assertions = [{
+      assertion = cfg.mem > 0;
+      message = "claude-vm.agent.mem must be greater than 0";
+    }];
+
     vcpu = lib.mkOption {
       type = lib.types.int;
       default = 4;
       description = "VM vCPU count. Overridable at runtime via VM_VCPU env var.";
     };
+    # Validate vcpu is positive  
+    assertions = [{
+      assertion = cfg.vcpu > 0;
+      message = "claude-vm.agent.vcpu must be greater than 0";
+    }];

Alternatively, use a more restrictive type:

     mem = lib.mkOption {
-      type = lib.types.int;
+      type = lib.types.ints.positive;
       default = 8192;
       description = "VM memory in MB. Overridable at runtime via VM_MEM env var.";
     };
     vcpu = lib.mkOption {
-      type = lib.types.int;
+      type = lib.types.ints.positive;
       default = 4;
       description = "VM vCPU count. Overridable at runtime via VM_VCPU env var.";
     };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mem = lib.mkOption {
type = lib.types.int;
default = 8192;
description = "VM memory in MB. Overridable at runtime via VM_MEM env var.";
};
vcpu = lib.mkOption {
type = lib.types.int;
default = 4;
description = "VM vCPU count. Overridable at runtime via VM_VCPU env var.";
};
mem = lib.mkOption {
type = lib.types.ints.positive;
default = 8192;
description = "VM memory in MB. Overridable at runtime via VM_MEM env var.";
};
vcpu = lib.mkOption {
type = lib.types.ints.positive;
default = 4;
description = "VM vCPU count. Overridable at runtime via VM_VCPU env var.";
};
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@modules/base.nix` around lines 25 - 34, The mem and vcpu options currently
accept any integer (including <=0); add an option-level validation on both the
mem and vcpu lib.mkOption blocks to reject non-positive values (e.g. validation
= v: if v > 0 then true else "mem must be a positive integer (MB)" and similarly
for vcpu: "vcpu must be a positive integer"); update the error strings to
clearly name the option being validated so configuration-time errors are
helpful.

};

config = {
Expand All @@ -31,8 +41,8 @@ in

microvm = {
hypervisor = "qemu";
mem = 4096;
vcpu = 4;
mem = cfg.mem;
vcpu = cfg.vcpu;

writableStoreOverlay = "/nix/.rw-store";

Expand Down
Loading