lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(bug-1560): symlink-escape test asserted the wrong error string

Browse source 
test_recordstart_symlink_escape_replies_error expected "invalid record
file name" (the -EINVAL string), but sched_validate_record_filename()'s
canonical-path containment check has always, correctly, returned
-EACCES ("permission denied accessing record file") for a symlink
escaping record_dir -- confirmed against test_recording_path_validation.c's
own direct unit test for the identical scenario, which already asserts
-EACCES and has always passed.

Not environment-dependent as originally suspected (this project's
buglog carried it as a uid-1000-vs-root permission-check-ordering
theory across 7+ sightings since Aug 15) -- it fails identically
regardless of uid, since escape-containment and filesystem-permission
errors were never actually conflated in production code, only in this
test's assertion. No production code changed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA
This commit is contained in:
Rongsong Shen 2026-08-30 15:31:45 +00:00
parent 6326109798
commit dfcd345ded
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
1 changed files with 21 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

23
src/scheduler/testsuite/test_admin_request.c
View file

@ -826,7 +826,25 @@ static void test_recordstart_symlink_escape_replies_error(void)
* '/' nor the ".." rule -- only the canonical-path containment check
* on the pre-existing symlink's resolved target catches this. Proven
* here through the real admin request path, not just at the internal
* sched_validate_record_filename() level. */
* sched_validate_record_filename() level.
*
* bug-1560: a canonical-path containment failure is deliberately
* -EACCES, not -EINVAL (sched_validate_record_filename(),
* recording.c's path_is_contained() branch) -- -EINVAL is reserved
* for a malformed filename rejected before any filesystem access
* (Step 1: '/' or ".." in the bare filename, see
* test_recordstart_dotdot_filename_replies_error above). The sibling
* unit test at the sched_validate_record_filename() level already
* asserts -EACCES for this exact scenario
* (test_symlink_inside_record_dir_escaping_outside_is_rejected,
* test_recording_path_validation.c) -- this test's own expected
* error string below was simply wrong, asserting the -EINVAL string
* against what is, and always was, an -EACCES rejection. Not an
* environment-dependent permission-check ordering issue as
* originally suspected: this fails identically as uid 0 or uid
* 1000, since escape-containment and filesystem-permission errors
* were never actually conflated in production code -- only in this
* test's assertion. */
snprintf(outside_target, sizeof(outside_target), "%s/secret", outside_dir);
touch_file(outside_target);
@ -847,7 +865,8 @@ static void test_recordstart_symlink_escape_replies_error(void)
TEST_ASSERT_EQUAL_INT(1, g_reply_calls);
TEST_ASSERT_FALSE(g_last_ok);
TEST_ASSERT_FALSE(g_last_error_null);
TEST_ASSERT_EQUAL_STRING("invalid record file name", g_last_error);
TEST_ASSERT_EQUAL_STRING("permission denied accessing record file",
g_last_error);
TEST_ASSERT_FALSE(g_last_recording);
TEST_ASSERT_TRUE(g_last_file_null);
TEST_ASSERT_EQUAL_UINT64(0, g_last_event_count);