Skip to content

fix(directory): read recursive flag as mrb_bool to prevent unintended recursive delete - #30

Open
ratazzi wants to merge 1 commit into
masterfrom
fix/directory-recursive-bool
Open

fix(directory): read recursive flag as mrb_bool to prevent unintended recursive delete#30
ratazzi wants to merge 1 commit into
masterfrom
fix/directory-recursive-bool

Conversation

@ratazzi

@ratazzi ratazzi commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Problem

mrb_get_args format b writes a single mrb_bool (u8), but recursive_val was declared as mrb_value (8 bytes, undefined-initialized). mrb_test() then read the 7 uninitialized high bytes, so the flag evaluated as true regardless of what the Ruby DSL passed.

A plain

directory "/some/dir" do
  action :delete
end

(recursive defaults to false) could therefore recursively delete the whole subtree instead of failing on a non-empty directory — silent data loss.

This is the same b-format pitfall previously fixed in macos_dock.zig; this call site was missed.

Fix

Declare the variable as mruby.mrb_bool and compare against zero, matching the existing pattern in remote_file.zig. Swept the repo for other b format specifiers paired with mrb_value — this was the only remaining one.

Testing

Verified end to end with the built binary:

  • Non-recursive action :delete on a non-empty directory now fails with DirNotEmpty and leaves the tree intact (previously deleted it).
  • recursive true still deletes the tree.
  • zig build test passes.

… recursive delete

mrb_get_args format `b` writes a single mrb_bool (u8), but recursive_val
was declared as mrb_value (8 bytes, undefined). mrb_test() then read the
7 uninitialized high bytes, so the flag evaluated as true regardless of
what the Ruby DSL passed. A plain

    directory "/some/dir" do
      action :delete
    end

(recursive defaults to false) could therefore recursively delete the
whole subtree instead of failing on a non-empty directory -- silent
data loss.

Declare the variable as mrb_bool and compare against zero, matching the
existing pattern in remote_file.zig. Swept the repo for other `b`
format specifiers paired with mrb_value: this was the only one.

Verified end to end: non-recursive delete of a non-empty directory now
fails with DirNotEmpty and leaves the tree intact; recursive true still
deletes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant