From dfcd345ded2eb52964d119b53a204684e1f15de9 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Sun, 30 Aug 2026 15:31:45 +0000 Subject: [PATCH] fix(bug-1560): symlink-escape test asserted the wrong error string 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 Claude-Session: https://claude.ai/code/session_019QxaSzHRB7pmYFp4cVnwkA --- src/scheduler/testsuite/test_admin_request.c | 23 ++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/scheduler/testsuite/test_admin_request.c b/src/scheduler/testsuite/test_admin_request.c index e135b0df..2957e1f1 100644 --- a/src/scheduler/testsuite/test_admin_request.c +++ b/src/scheduler/testsuite/test_admin_request.c @@ -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);