Skip to content

Global --dry-run is ignored; destructive commands still execute with --force #17

Description

@nexus-labs-admin

Summary

The global --dry-run flag is parsed and copied into RunContext, but it is never consulted before command dispatch and no command reads ctx.dry_run.

As a result, combining --dry-run with the required confirmation flag does not prevent mutations. For example:

lox --dry-run config push config.Loxone --reboot --force

follows the normal push path: it validates, uploads /prog/sps_new.zip, and triggers a fast reload or reboot. The same problem affects the other mutating commands whose only guard is --force or --write.

This was found by source inspection; I deliberately did not execute the destructive reproduction against a Miniserver.

Evidence

At current main (bd179bafc2f0d6640e9a9d80adef2cad9dc99221):

  • Cli::dry_run is copied to RunContext::dry_run in src/main.rs.
  • RunContext::dry_run is marked #[allow(dead_code)] in src/commands/mod.rs.
  • Searching src/ for dry_run finds only the declarations/assignment and test-context initializers; there is no behavioral check.
  • ConfigCmd::Push, Upload, Patch, PushHttp, and Restore check --force, but not ctx.dry_run.

The published v0.12.0 source has the same pattern: only the CLI declaration, context assignment, context field, and test initializers reference dry_run.

This conflicts with the documented contract in COMMANDS.md: “--dry-run — Validate and resolve inputs without executing commands.”

Impact

This is particularly dangerous for configuration-management integrations. An Ansible check-mode wrapper could reasonably invoke the normal non-interactive command with both --force and --dry-run, expecting a plan, and instead replace or reload the live building configuration.

The flag can also mislead users around local mutations, not only Miniserver writes.

Expected behavior

With --dry-run, commands should:

  1. resolve and validate all inputs;
  2. report the exact intended action in table/JSON output;
  3. perform no local writes, FTP/HTTP uploads, live state writes, reloads, or reboots;
  4. have tests proving that mutating backends are not called even when --force or --write is also present.

If full planning support is not yet available for a command, it would be safer for that command to fail explicitly with “dry-run not supported” than to continue normally.

Suggested regression coverage

Add mocked-backend tests for at least:

  • --dry-run config push ... --force
  • --dry-run config upload ... --force
  • --dry-run config patch ... --force
  • --dry-run config push-http ... --force
  • --dry-run config restore ... --force
  • --dry-run live set ... --write

Each test should assert zero network requests and zero filesystem mutations.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions