Skip to content

ENT-14297: Improve error message when cf-agent cannot write to an immutable file#6236

Draft
nickanderson wants to merge 2 commits into
cfengine:masterfrom
nickanderson:ENT-14297/master
Draft

ENT-14297: Improve error message when cf-agent cannot write to an immutable file#6236
nickanderson wants to merge 2 commits into
cfengine:masterfrom
nickanderson:ENT-14297/master

Conversation

@nickanderson

@nickanderson nickanderson commented Jul 14, 2026

Copy link
Copy Markdown
Member

Problem

When cf-agent cannot open a file for writing via the content attribute, the error gives no reason at all:

error: Cannot open file '/tmp/immutable.txt' for writing

The immutable bit (chattr +i) is frequently set deliberately to keep CFEngine from managing a file, and this generic message forces operators to investigate the underlying cause.

Change

The actual system error is now always reported, and when the file carries the immutable bit (and it is not being overridden) that is called out explicitly:

error: Cannot open file '/tmp/immutable.txt' for writing: Operation not permitted (the immutable bit is set)

Other failures now include the system error string, which the previous message omitted entirely:

error: Cannot open file '/etc/foo' for writing: Read-only file system

Jira: ENT-14297

@larsewi larsewi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This only improves the message for when you are using the content attribute. Adding these checks everywhere would be quite tedious. Maybe it would be better to have OverrideImmutableBegin() take care of this logic?

@nickanderson

Copy link
Copy Markdown
Member Author

This only improves the message for when you are using the content attribute. Adding these checks everywhere would be quite tedious. Maybe it would be better to have OverrideImmutableBegin() take care of this logic?

WTH, not all my commits are here and I thought they were, sorry. will get them up later

@nickanderson nickanderson marked this pull request as draft July 15, 2026 14:42
…-14297)

Covers the cases where cf-agent cannot write to a file because the
immutable bit is set and the bit is not being overridden:
  - content attribute (14, fails at open)
  - edit_line         (15, fails at the temp->target rename)
  - copy_from         (16, fails at the temp->target rename)

Each failing promise runs in a sub policy so its non-zero exit code does
not fail the test; the test asserts on the captured error output. These
tests fail until the accompanying error-message improvement is in place.
…4297)

When cf-agent could not write a file whose immutable bit is set, the
error gave no reason. Now the actual system error is always reported, and
the immutable bit is called out explicitly when it is set (and not being
overridden):

    error: Cannot open file '/tmp/x' for writing: Operation not permitted (the immutable bit is set)
    error: Failed to replace original file '/tmp/x' with copy '...': Operation not permitted (the immutable bit is set)

The immutable annotation is confirmed via FSAttrsGetImmutableFlag(), not
guessed from errno (an immutable file typically fails with EPERM, but
that is not exclusive to immutability), so other causes (permissions, MAC
policy, a read-only or full filesystem) are never mislabeled.

The check lives at the two points where the write actually fails: the
in-place open in WriteContentFromString() (content), and the temporary-
file rename in OverrideImmutableRename(), which is the common landing
point for edit_line, edit_xml, copy_from and templating. It is skipped
when overriding the immutable bit, since then the agent writes to a
temporary copy that is renamed into place after clearing the bit.
@nickanderson

Copy link
Copy Markdown
Member Author

Good call — the previous version only covered the content attribute.

I looked at generalizing via OverrideImmutableBegin(), but that helper is only used by the content path. The shared landing point for edit_line, edit_xml, copy_from and both template engines is OverrideImmutableRename() — they all write a temporary file via SaveAsFile() and rename it over the target — so I moved the detection there. That covers all of them in one place without repeating the check at each call site.

The content path writes in place rather than renaming, so it keeps its own detection at the fopen() failure. Two points total:

  • WriteContentFromString() — the in-place open (content)
  • OverrideImmutableRename() — the temp→target rename (edit_line, edit_xml, copy_from, templating)

Both confirm the bit via FSAttrsGetImmutableFlag() rather than guessing from errno, and both are skipped when overriding (that path clears the bit and renames a temp copy into place).

Added acceptance tests for edit_line and copy_from (15/16) alongside content (14). Verified end-to-end against a real immutable file (KVM VM, ext4 root):

content:   Cannot open file '/c.txt' for writing: Operation not permitted (the immutable bit is set)
edit_line: Failed to replace original file '/e.txt' with copy '...': Operation not permitted (the immutable bit is set)
copy_from: Failed to replace original file '/d.txt' with copy '...': Operation not permitted (the immutable bit is set)

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants