Skip to content

fix(cli): export --account writes the .mac file with 0600 permissions - #2469

Open
ygd58 wants to merge 1 commit into
0xMiden:nextfrom
ygd58:fix-export-account-file-permissions
Open

fix(cli): export --account writes the .mac file with 0600 permissions#2469
ygd58 wants to merge 1 commit into
0xMiden:nextfrom
ygd58:fix-export-account-file-permissions

Conversation

@ygd58

@ygd58 ygd58 commented Aug 29, 2026

Copy link
Copy Markdown

export_account writes AccountFile (whose auth_secret_keys field carries the account's raw secret keys) to disk via plain File::create(), which on Unix creates the file with the process's default umask permissions (typically 0644 - world-readable). Any local user on a shared machine could read another user's exported account secret keys from this file.

Context

#1833 (merged) already audited and fixed this exact class of issue for the filesystem keystore (crates/rust-client/src/keystore/fs_keystore.rs) and the SQLite database (crates/sqlite-store/src/db_management/pool_manager.rs), but didn't cover this CLI export command, which writes the same kind of secret key material to an arbitrary, user-chosen path.

Fix

Adds two small helpers mirroring the fix already applied to fs_keystore.rs (see #2468): create_secret_data_file opens with OpenOptions::mode(0o600) on Unix so the file is 0600 from creation, and restrict_secret_data_file_permissions calls set_permissions(0o600) explicitly afterwards, since OpenOptions::mode only applies when a file is newly created - it has no effect if a file already existed at that path (e.g. a stale export left over from a previous run) and is truncated and reopened for writing instead. export_note (which writes public note data, not secret keys) is left unchanged.

Test plan

Added two unit tests on the helpers directly, without needing the full async Client/keystore setup export_account itself requires: one asserting a freshly created file is 0600, one asserting a pre-existing 0644 file still ends up 0600 after being overwritten.

Ran locally, not just CI:

cargo test -p miden-client-cli newly_created_secret_data_file_is_0600 -- --nocapture
cargo test -p miden-client-cli overwriting_an_existing_export_file_still_ends_up_0600 -- --nocapture
cargo test -p miden-client-cli --lib

Both new tests: ok. Full lib suite: 8 passed, 0 failed - no regressions.

export_account writes AccountFile (whose auth_secret_keys field carries
the account's raw secret keys) to disk via plain File::create(), which on
Unix creates the file with the process's default umask permissions
(typically 0644 - world-readable). Any local user on a shared machine
could read another user's exported account secret keys from this file.

0xMiden#1833 (merged) already audited and fixed this exact class of issue for
the filesystem keystore (crates/rust-client/src/keystore/fs_keystore.rs)
and the SQLite database (crates/sqlite-store/src/db_management/
pool_manager.rs), but didn't cover this CLI export command, which writes
the same kind of secret key material to an arbitrary, user-chosen path.

Adds two small helpers mirroring the fix already applied to
fs_keystore.rs: create_secret_data_file opens with OpenOptions::mode(
0o600) on Unix so the file is 0600 from creation, and
restrict_secret_data_file_permissions calls set_permissions(0o600)
explicitly afterwards, since OpenOptions::mode only applies when a file is
newly created - it has no effect if a file already existed at that path
(e.g. a stale export left over from a previous run) and is truncated and
reopened for writing instead. export_note (which writes public note data,
not secret keys) is left unchanged.

Adds two unit tests on the helpers directly, without needing the full
async Client/keystore setup export_account itself requires: one asserting
a freshly created file is 0600, one asserting a pre-existing 0644 file
still ends up 0600 after being overwritten.

I don't have a matching Rust toolchain in my current environment, so this
needs local build + test verification before being considered ready, same
as my prior PRs in this repo.
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