add a github check for programs not using traversal #8698

sylvestre · 2025-09-21T20:12:29Z

No description provided.

codspeed-hq · 2025-09-21T20:32:52Z

CodSpeed Performance Report

Merging #8698 will not alter performance

_{Comparing sylvestre:check-tra (da38534) with main (e4835ac)}

Summary

✅ 55 untouched
⏩ 73 skipped¹

73 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

github-actions · 2025-09-21T20:33:30Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-22T07:07:57Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-26T17:33:32Z

GNU testsuite comparison:

GNU test failed: tests/rm/cycle. tests/rm/cycle is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/empty-inacc. tests/rm/empty-inacc is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/r-1. tests/rm/r-1 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/r-2. tests/rm/r-2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/rm2. tests/rm/rm2 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/rm3. tests/rm/rm3 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/rm5. tests/rm/rm5 is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/unreadable. tests/rm/unreadable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/v-slash. tests/rm/v-slash is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/rm/rm1 (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-26T22:07:51Z

GNU testsuite comparison:

GNU test failed: tests/rm/ir-1. tests/rm/ir-1 is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions · 2025-09-26T23:08:14Z

GNU testsuite comparison:

GNU test failed: tests/rm/ir-1. tests/rm/ir-1 is passing on 'main'. Maybe you have to rebase?

github-actions · 2025-09-27T18:48:24Z

GNU testsuite comparison:

GNU test failed: tests/du/long-from-unreadable. tests/du/long-from-unreadable is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pwd/pwd-long. tests/pwd/pwd-long is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/rm/deep-2. tests/rm/deep-2 is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions · 2025-09-28T09:04:33Z

GNU testsuite comparison:

GNU test failed: tests/rm/ir-1. tests/rm/ir-1 is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-28T09:28:45Z

GNU testsuite comparison:

GNU test failed: tests/rm/ir-1. tests/rm/ir-1 is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-28T18:35:27Z

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-28T19:52:32Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

github-actions · 2025-09-29T20:12:49Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-30T06:01:07Z

GNU testsuite comparison:

GNU test failed: tests/rm/rm2. tests/rm/rm2 is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/rm/rm1 (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

github-actions · 2025-09-30T07:45:18Z

GNU testsuite comparison:

GNU test failed: tests/rm/rm2. tests/rm/rm2 is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/rm/rm1 (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

…ated file

github-actions · 2025-09-30T21:21:05Z

GNU testsuite comparison:

Skip an intermittent issue tests/rm/rm1 (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

cakebaker · 2025-10-01T14:27:45Z

util/check-safe-traversal.sh

+# Build utilities if not already built
+if [ ! -f "$PROJECT_ROOT/target/release/rm" ]; then
+    echo "Building utilities..."
+    cd "$PROJECT_ROOT"
+    cargo build --release --quiet
+fi
+
+# Check if we should use individual binaries or multicall binary
+# Prefer individual binaries for more accurate testing
+if [ -f "$PROJECT_ROOT/target/release/rm" ]; then
+    echo "Using individual binaries (preferred for testing)"
+    USE_MULTICALL=0
+elif [ -f "$PROJECT_ROOT/target/release/coreutils" ]; then
+    echo "Using multicall binary: $PROJECT_ROOT/target/release/coreutils"
+    USE_MULTICALL=1
+    COREUTILS_BIN="$PROJECT_ROOT/target/release/coreutils"
+else
+    echo "No binaries found - please build first"
+    exit 1
+fi


Is this script intended to be used outside of the CI? If not, you could omit these checks.

cakebaker · 2025-10-01T14:46:50Z

util/check-safe-traversal.sh

+
+# Check for dangerous patterns across all logs
+echo "Checking for dangerous path resolution patterns..."
+echo "✓ Basic safe traversal verification completed"


This output feels out of place. I guess it should be shown before the "Additional Safety Checks"?

cakebaker · 2025-10-01T15:04:43Z

tests/by-util/test_rm.rs

+        (stderr.contains("rm: cannot remove 'a/1': Directory not empty")
+            && stderr.contains("rm: cannot remove 'a': Directory not empty"))
+            || (stderr.contains("rm: cannot remove 'a/1/2': Permission denied")
+                && stderr.contains("rm: cannot remove 'b/3': Permission denied"))


What's the reason for the OR? I would expect that the command always returns the same error messages.

cakebaker · 2025-10-01T15:16:10Z

tests/by-util/test_rm.rs

+        stderr.contains("rm: cannot remove directory 'b/a/p': Permission denied")
+            || stderr.contains("rm: cannot remove 'b/a/p': Permission denied")


Same here, I don't understand why there is an OR.

cakebaker · 2025-10-01T15:39:36Z

src/uucore/src/lib/features/perms.rs

-            match wrap_chown(
+            // Use safe syscalls for root directory to prevent TOCTOU attacks
+            #[cfg(target_os = "linux")]
+            let chown_result = if path.is_dir() {


I think you could use cfg! in the condition. It would allow you to get rid of the duplicate block starting on line 335.

cakebaker · 2025-10-01T15:57:18Z

src/uucore/src/lib/features/perms.rs

+        dir_fd: &DirFd,
+        path: &Path,
+        meta: &Metadata,
+    ) -> Result<String, String> {


What's the reason for the String in the "ok" case? The function always returns an empty string in that case, so my suggestion is to use Result<(), String> as return type.

cakebaker

I will continue with the review of linux.rs and rm.rs tomorrow, the other files are reviewed.

sylvestre force-pushed the check-tra branch from 221e3b9 to 0881dcf Compare September 22, 2025 06:47

sylvestre force-pushed the check-tra branch from 0881dcf to f739e60 Compare September 26, 2025 17:13

sylvestre force-pushed the check-tra branch from f739e60 to f4bfb67 Compare September 26, 2025 21:47

sylvestre force-pushed the check-tra branch 3 times, most recently from 33ad553 to 5082e23 Compare September 26, 2025 22:47

sylvestre force-pushed the check-tra branch from 5082e23 to 2ce39de Compare September 27, 2025 18:27

sylvestre force-pushed the check-tra branch from 3ebac82 to 1376176 Compare September 28, 2025 09:05

sylvestre force-pushed the check-tra branch from 42d619c to 7bbd68d Compare September 28, 2025 19:32

sylvestre force-pushed the check-tra branch from 7bbd68d to 210cf9a Compare September 29, 2025 19:48

sylvestre marked this pull request as ready for review September 29, 2025 19:48

sylvestre force-pushed the check-tra branch from 210cf9a to 0ff9bb8 Compare September 30, 2025 05:39

sylvestre force-pushed the check-tra branch from 0ff9bb8 to e9aa7c5 Compare September 30, 2025 07:19

sylvestre added 3 commits September 30, 2025 22:48

chmod: on linux use the safe traversal functions

77d38f8

safe-traversal is always used on linux, adjust the cfg

e0bcb17

add a github check for programs not using traversal

95cf493

sylvestre added 2 commits September 30, 2025 22:48

rm: on linux use the safe traversal in all cases

a0bd693

rm: remove the unsafe code and move the rm linux functions in a dedic…

85f3380

…ated file

sylvestre force-pushed the check-tra branch from bd0ab13 to d00a2c9 Compare September 30, 2025 20:48

Fix the last rm tests + add tests

da38534

sylvestre force-pushed the check-tra branch from d00a2c9 to da38534 Compare September 30, 2025 20:57

sylvestre requested a review from cakebaker October 1, 2025 05:50

cakebaker reviewed Oct 1, 2025

View reviewed changes

		stderr.contains("rm: cannot remove directory 'b/a/p': Permission denied")
		\|\| stderr.contains("rm: cannot remove 'b/a/p': Permission denied")

Uh oh!

add a github check for programs not using traversal #8698

Are you sure you want to change the base?

add a github check for programs not using traversal #8698

Conversation

sylvestre commented Sep 21, 2025

Uh oh!

codspeed-hq bot commented Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #8698 will not alter performance

Summary

Footnotes

Uh oh!

github-actions bot commented Sep 21, 2025

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

github-actions bot commented Sep 26, 2025

Uh oh!

github-actions bot commented Sep 26, 2025

Uh oh!

github-actions bot commented Sep 26, 2025

Uh oh!

github-actions bot commented Sep 27, 2025

Uh oh!

github-actions bot commented Sep 28, 2025

Uh oh!

github-actions bot commented Sep 28, 2025

Uh oh!

github-actions bot commented Sep 28, 2025

Uh oh!

github-actions bot commented Sep 28, 2025

Uh oh!

github-actions bot commented Sep 29, 2025

Uh oh!

github-actions bot commented Sep 30, 2025

Uh oh!

github-actions bot commented Sep 30, 2025

Uh oh!

github-actions bot commented Sep 30, 2025

Uh oh!

cakebaker Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

cakebaker Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

cakebaker Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

cakebaker Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

cakebaker Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

cakebaker Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

cakebaker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codspeed-hq bot commented Sep 21, 2025 •

edited

Loading