Skip to content

Make config writes atomic and surface a diff when they fail#536

Merged
bmcnama-shopify merged 1 commit intomainfrom
config-atomic-write-with-diff
Apr 30, 2026
Merged

Make config writes atomic and surface a diff when they fail#536
bmcnama-shopify merged 1 commit intomainfrom
config-atomic-write-with-diff

Conversation

@bmcnama-shopify
Copy link
Copy Markdown
Contributor

@bmcnama-shopify bmcnama-shopify commented Apr 29, 2026

Make CLI::Kit::Config writes atomic (tmpfile + rename) and surface a
diff of only the keys that changed when the write fails, so users whose
~/.config/<tool>/config is read-only — for example a nix/home-manager-
managed symlink into /nix/store — can see exactly what the tool was
trying to change instead of just getting a bare Errno::EACCES.

Atomic writes

write_config now writes to a Tempfile in the same directory and
renames it into place, so readers never observe a partial write. The
existing file mode is preserved across the rename, and new configs are
created with the umask-adjusted default (0o666 & ~File.umask,
matching open(2)); Tempfile defaults to 0o600, which would
otherwise both silently tighten permissions on an existing config and
create new configs with that more restrictive mode.

Symlink preservation

When the config path is a symlink, the target is resolved before
writing so the rename replaces the real file at the end of the chain
instead of clobbering the symlink with a regular file. This preserves
nix/home-manager-managed configs and surfaces EACCES/EROFS
naturally when the target is read-only, instead of silently replacing
a managed symlink with a local copy that future config-management
updates would stop applying to.

ConfigWriteError

ConfigWriteError is a new error class raised on EACCES, EPERM,
and EROFS. It inherits from SystemCallError so existing
rescue SystemCallError handlers still match, and it preserves the
original errno from the wrapped exception. The message includes a
section-scoped diff of only the keys that actually changed; unchanged
keys (which may include sensitive values such as API tokens) are
deliberately omitted so the failure message can never leak secrets
through stderr or exception reports.

Example error output:

Could not write to /Users/you/.config/tool/config: Permission denied

Attempted changes (unchanged keys omitted):
  [hooks]
- path_check_enabled = true
+ path_check_enabled = false

Tests cover atomic-write correctness, tmpfile cleanup, new-file mode
following the umask, preservation of an existing file's mode, diff on
a read-only directory, new-section diff, secret-leak protection,
EROFS (read-only filesystem), rescue SystemCallError
compatibility, and symlink preservation on both successful and failing
writes.

Copy link
Copy Markdown
Contributor

@joshheinrichs-shopify joshheinrichs-shopify left a comment

Choose a reason for hiding this comment

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

lgtm aside from the perms thing on new files thanks brandon!

Comment thread lib/cli/kit/config.rb Outdated
Make `CLI::Kit::Config` writes atomic (tmpfile + rename) and surface a
diff of only the keys that changed when the write fails, so users whose
`~/.config/<tool>/config` is read-only — for example a nix/home-manager-
managed symlink into `/nix/store` — can see exactly what the tool was
trying to change instead of just getting a bare `Errno::EACCES`.

Atomic writes
-------------
`write_config` now writes to a `Tempfile` in the same directory and
renames it into place, so readers never observe a partial write. The
existing file mode is preserved across the rename, and new configs are
created with the umask-adjusted default (`0o666 & ~File.umask`,
matching `open(2)`); `Tempfile` defaults to `0o600`, which would
otherwise both silently tighten permissions on an existing config and
create new configs with that more restrictive mode.

Symlink preservation
--------------------
When the config path is a symlink, the target is resolved before
writing so the rename replaces the real file at the end of the chain
instead of clobbering the symlink with a regular file. This preserves
nix/home-manager-managed configs and surfaces `EACCES`/`EROFS`
naturally when the target is read-only, instead of silently replacing
a managed symlink with a local copy that future config-management
updates would stop applying to.

ConfigWriteError
----------------
`ConfigWriteError` is a new error class raised on `EACCES`, `EPERM`,
and `EROFS`. It inherits from `SystemCallError` so existing
`rescue SystemCallError` handlers still match, and it preserves the
original errno from the wrapped exception. The message includes a
section-scoped diff of only the keys that actually changed; unchanged
keys (which may include sensitive values such as API tokens) are
deliberately omitted so the failure message can never leak secrets
through stderr or exception reports.

Example error output:

    Could not write to /Users/you/.config/tool/config: Permission denied

    Attempted changes (unchanged keys omitted):
      [hooks]
    - path_check_enabled = true
    + path_check_enabled = false

Tests cover atomic-write correctness, tmpfile cleanup, new-file mode
following the umask, preservation of an existing file's mode, diff on
a read-only directory, new-section diff, secret-leak protection,
`EROFS` (read-only filesystem), `rescue SystemCallError`
compatibility, and symlink preservation on both successful and failing
writes.
@bmcnama-shopify bmcnama-shopify force-pushed the config-atomic-write-with-diff branch from f6365ec to 377b8d6 Compare April 29, 2026 23:10
@bmcnama-shopify bmcnama-shopify merged commit 4df1dd4 into main Apr 30, 2026
21 checks passed
@bmcnama-shopify bmcnama-shopify deleted the config-atomic-write-with-diff branch April 30, 2026 00:22
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.

2 participants