Skip to content

Commit 18b0f91

Browse files
nickandersonnickanderson
authored
Merge pull request #6236 from nickanderson/ENT-14297/master
ENT-14297: Improve error message when cf-agent cannot write to an immutable file
2 parents 8c26d9c + b9cab70 commit 18b0f91

16 files changed

Lines changed: 827 additions & 6 deletions

‎cf-agent/verify_files.c‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -835,7 +835,29 @@ static PromiseResult WriteContentFromString(EvalContext *ctx, const char *path,
835835
FILE *f = safe_fopen(override_path, "w");
836836
if (f == NULL)
837837
{
838-
RecordFailure(ctx, pp, attr, "Cannot open file '%s' for writing", path);
838+
const int saved_errno = errno;
839+
840+
/* Always report the actual system error. When not overriding the
841+
* immutable bit we write in place, so also flag it if the file is
842+
* immutable - confirmed via the flag, not guessed from errno (which
843+
* is EPERM but not exclusive to immutability), so other causes are
844+
* not mislabeled. When overriding we write to a temporary copy, so
845+
* the immutable bit is not the cause. */
846+
bool is_immutable = false;
847+
if (!override_immutable
848+
&& (FSAttrsGetImmutableFlag(changes_path, &is_immutable) == FS_ATTRS_SUCCESS)
849+
&& is_immutable)
850+
{
851+
RecordFailure(ctx, pp, attr,
852+
"Cannot open file '%s' for writing: %s (the immutable bit is set)",
853+
path, GetErrorStrFromCode(saved_errno));
854+
}
855+
else
856+
{
857+
RecordFailure(ctx, pp, attr,
858+
"Cannot open file '%s' for writing: %s",
859+
path, GetErrorStrFromCode(saved_errno));
860+
}
839861
OverrideImmutableCommit(changes_path, override_path, override_immutable, true);
840862
return PromiseResultUpdate(result, PROMISE_RESULT_FAIL);
841863
}

‎libpromises/override_fsattrs.c‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,33 @@ bool OverrideImmutableRename(
211211

212212
if (rename(old_filename, new_filename) == -1)
213213
{
214-
Log(LOG_LEVEL_ERR,
215-
"Failed to replace original file '%s' with copy '%s': %s",
216-
new_filename,
217-
old_filename,
218-
GetErrorStr());
214+
const int saved_errno = errno;
215+
216+
/* This is the common landing point for edit_line, edit_xml, copy_from
217+
* and templating (they write a temporary file and rename it into
218+
* place). When not overriding the immutable bit, an immutable
219+
* destination is the likely reason the rename failed, so flag it -
220+
* confirmed via the flag, not guessed from errno, so other causes are
221+
* not mislabeled. */
222+
bool is_immutable = false;
223+
if (!override
224+
&& (FSAttrsGetImmutableFlag(new_filename, &is_immutable) == FS_ATTRS_SUCCESS)
225+
&& is_immutable)
226+
{
227+
Log(LOG_LEVEL_ERR,
228+
"Failed to replace original file '%s' with copy '%s': %s (the immutable bit is set)",
229+
new_filename,
230+
old_filename,
231+
GetErrorStrFromCode(saved_errno));
232+
}
233+
else
234+
{
235+
Log(LOG_LEVEL_ERR,
236+
"Failed to replace original file '%s' with copy '%s': %s",
237+
new_filename,
238+
old_filename,
239+
GetErrorStrFromCode(saved_errno));
240+
}
219241
unlink(old_filename);
220242
return false;
221243
}

‎tests/acceptance/10_files/unsafe/14_immutable_content.cf‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
##############################################################################
2+
#
3+
# Test that agent reports the immutable bit in the error message when it
4+
# cannot write to an immutable file (content attribute, no override).
5+
#
6+
# The whole scenario (create the file, set it immutable, then fail to write it)
7+
# runs in a self-contained sub policy: its setup happens in its own agent
8+
# process, so it does not depend on this test's evaluation order, and its
9+
# non-zero exit does not fail this test. We only assert on the captured error
10+
# output. Teardown clears the immutable bit natively via body fsattrs.
11+
#
12+
##############################################################################
13+
body common control
14+
{
15+
inputs => { "../../default.sub.cf" };
16+
bundlesequence => { default("$(this.promise_filename)") };
17+
version => "1.0";
18+
}
19+
20+
bundle agent global
21+
{
22+
vars:
23+
"testfile" string => "/tmp/14_immutable.txt";
24+
}
25+
26+
body fsattrs clear_immutable
27+
{
28+
immutable => "false";
29+
}
30+
31+
bundle agent test
32+
{
33+
meta:
34+
"description" -> { "ENT-14297" }
35+
string => "Test that the error message names the immutable bit when agent cannot write to an immutable file";
36+
37+
"test_skip_unsupported" string => "hpux|aix|solaris|windows";
38+
}
39+
40+
bundle agent check
41+
{
42+
vars:
43+
"run"
44+
data => execresult_as_data(
45+
"$(sys.cf_agent) -KI -f $(with)", "noshell", "both"
46+
),
47+
with => regex_replace("$(this.promise_filename)", "\.cf$", ".sub.cf", "");
48+
49+
classes:
50+
"ok"
51+
expression => regcmp(
52+
".*Cannot open file.*the immutable bit is set.*", "$(run[output])"
53+
);
54+
55+
reports:
56+
ok::
57+
"$(this.promise_filename) Pass";
58+
59+
!ok::
60+
"$(this.promise_filename) FAIL";
61+
62+
DEBUG::
63+
"$(this.bundle): agent exited '$(run[exit_code])', output was:";
64+
"$(run[output])";
65+
}
66+
67+
bundle agent destroy
68+
{
69+
files:
70+
"$(global.testfile)"
71+
delete => tidy,
72+
fsattrs => clear_immutable;
73+
}

‎tests/acceptance/10_files/unsafe/14_immutable_content.sub.cf‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
body common control
2+
{
3+
bundlesequence => { "setup", "try_modify" };
4+
}
5+
6+
body fsattrs set_immutable
7+
{
8+
immutable => "true";
9+
}
10+
11+
bundle agent setup
12+
{
13+
files:
14+
# Create the target and set the immutable bit natively. This runs in its
15+
# own agent process (invoked from the test's check bundle), so setup is
16+
# ordered before the write below and does not depend on the parent test.
17+
"/tmp/14_immutable.txt"
18+
content => "original content",
19+
fsattrs => set_immutable;
20+
}
21+
22+
bundle agent try_modify
23+
{
24+
files:
25+
# No fsattrs => ... immutable, so the immutable bit is not overridden.
26+
# The content differs from what setup wrote, so the agent tries to open the
27+
# immutable file for writing and fails.
28+
"/tmp/14_immutable.txt"
29+
content => "the agent tries to modify an immutable file";
30+
}

‎tests/acceptance/10_files/unsafe/15_immutable_edit_line.cf‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
##############################################################################
2+
#
3+
# Test that agent reports the immutable bit in the error message when it
4+
# cannot write to an immutable file via edit_line (no override).
5+
#
6+
# edit_line writes a temporary file and renames it over the target, so the
7+
# failure surfaces from OverrideImmutableRename(). The whole scenario (create
8+
# the file, set it immutable, then fail to edit it) runs in a self-contained
9+
# sub policy: its setup happens in its own agent process, so it does not depend
10+
# on this test's evaluation order, and its non-zero exit does not fail this
11+
# test. We only assert on the captured error output. Teardown clears the
12+
# immutable bit natively via body fsattrs.
13+
#
14+
##############################################################################
15+
body common control
16+
{
17+
inputs => { "../../default.sub.cf" };
18+
bundlesequence => { default("$(this.promise_filename)") };
19+
version => "1.0";
20+
}
21+
22+
bundle agent global
23+
{
24+
vars:
25+
"testfile" string => "/tmp/15_immutable.txt";
26+
}
27+
28+
body fsattrs clear_immutable
29+
{
30+
immutable => "false";
31+
}
32+
33+
bundle agent test
34+
{
35+
meta:
36+
"description" -> { "ENT-14297" }
37+
string => "Test that edit_line on an immutable file names the immutable bit in the error";
38+
39+
"test_skip_unsupported" string => "hpux|aix|solaris|windows";
40+
}
41+
42+
bundle agent check
43+
{
44+
vars:
45+
"run"
46+
data => execresult_as_data(
47+
"$(sys.cf_agent) -KI -f $(with)", "noshell", "both"
48+
),
49+
with => regex_replace("$(this.promise_filename)", "\.cf$", ".sub.cf", "");
50+
51+
classes:
52+
"ok"
53+
expression => regcmp(
54+
".*Failed to replace.*the immutable bit is set.*", "$(run[output])"
55+
);
56+
57+
reports:
58+
ok::
59+
"$(this.promise_filename) Pass";
60+
61+
!ok::
62+
"$(this.promise_filename) FAIL";
63+
64+
DEBUG::
65+
"$(this.bundle): agent exited '$(run[exit_code])', output was:";
66+
"$(run[output])";
67+
}
68+
69+
bundle agent destroy
70+
{
71+
files:
72+
"$(global.testfile)"
73+
delete => tidy,
74+
fsattrs => clear_immutable;
75+
76+
"$(global.testfile).cf-before-edit" delete => tidy;
77+
}

‎tests/acceptance/10_files/unsafe/15_immutable_edit_line.sub.cf‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
body common control
2+
{
3+
bundlesequence => { "setup", "try_modify" };
4+
}
5+
6+
body fsattrs set_immutable
7+
{
8+
immutable => "true";
9+
}
10+
11+
bundle edit_line insert_a_line
12+
{
13+
insert_lines:
14+
"a line the agent tries to insert into an immutable file";
15+
}
16+
17+
bundle agent setup
18+
{
19+
files:
20+
# Create the target and set the immutable bit natively. This runs in its
21+
# own agent process (invoked from the test's check bundle), so setup is
22+
# ordered before the edit below and does not depend on the parent test.
23+
"/tmp/15_immutable.txt"
24+
content => "original line",
25+
fsattrs => set_immutable;
26+
}
27+
28+
bundle agent try_modify
29+
{
30+
files:
31+
# No fsattrs => ... immutable, so the immutable bit is not overridden.
32+
# Inserting a new line changes the file, so the agent writes a temporary
33+
# file and renames it over the immutable target, which fails.
34+
"/tmp/15_immutable.txt" edit_line => insert_a_line;
35+
}

‎tests/acceptance/10_files/unsafe/16_immutable_copy_from.cf‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
##############################################################################
2+
#
3+
# Test that agent reports the immutable bit in the error message when it
4+
# cannot write to an immutable file via copy_from (no override).
5+
#
6+
# copy_from writes a temporary file and renames it over the target, so the
7+
# failure surfaces from OverrideImmutableRename(). The whole scenario (create
8+
# source + immutable destination, then fail to copy) runs in a self-contained
9+
# sub policy: its setup happens in its own agent process, so it does not depend
10+
# on this test's evaluation order, and its non-zero exit does not fail this
11+
# test. We only assert on the captured error output. Teardown clears the
12+
# immutable bit natively via body fsattrs.
13+
#
14+
##############################################################################
15+
body common control
16+
{
17+
inputs => { "../../default.sub.cf" };
18+
bundlesequence => { default("$(this.promise_filename)") };
19+
version => "1.0";
20+
}
21+
22+
bundle agent global
23+
{
24+
vars:
25+
"source" string => "/tmp/16_immutable.src";
26+
"testfile" string => "/tmp/16_immutable.txt";
27+
}
28+
29+
body fsattrs clear_immutable
30+
{
31+
immutable => "false";
32+
}
33+
34+
bundle agent test
35+
{
36+
meta:
37+
"description" -> { "ENT-14297" }
38+
string => "Test that copy_from onto an immutable file names the immutable bit in the error";
39+
40+
"test_skip_unsupported" string => "hpux|aix|solaris|windows";
41+
}
42+
43+
bundle agent check
44+
{
45+
vars:
46+
"run"
47+
data => execresult_as_data(
48+
"$(sys.cf_agent) -KI -f $(with)", "noshell", "both"
49+
),
50+
with => regex_replace("$(this.promise_filename)", "\.cf$", ".sub.cf", "");
51+
52+
classes:
53+
"ok"
54+
expression => regcmp(
55+
".*Failed to replace.*the immutable bit is set.*", "$(run[output])"
56+
);
57+
58+
reports:
59+
ok::
60+
"$(this.promise_filename) Pass";
61+
62+
!ok::
63+
"$(this.promise_filename) FAIL";
64+
65+
DEBUG::
66+
"$(this.bundle): agent exited '$(run[exit_code])', output was:";
67+
"$(run[output])";
68+
}
69+
70+
bundle agent destroy
71+
{
72+
files:
73+
"$(global.testfile)"
74+
delete => tidy,
75+
fsattrs => clear_immutable;
76+
77+
"$(global.source)" delete => tidy;
78+
}

0 commit comments

Comments
 (0)