On 23/10/2026 08:03, Collin Funk wrote:
Should I add a news entry for this one? I imagine the error is rarely
seen since, in practice, it only shows up when you are root and
fchownat fails, which should be rare.
I was tempted to skip adding a test for it, but I figured making
excuses not to write tests is a bad habit. :)
-- 8< --
* src/copy.c (copy_internal): Add a missing quoteaf call.
* tests/cp/link-no-deref.sh: Invoke getlimits_ and uses_strace_. Inject
an error using 'strace' to verify the name of the destination is quoted
in the error message.
Discovered by Anthropic's OSS Scanner, as bug ANT-2026-3M3N6L8K.
---
src/copy.c | 2 +-
tests/cp/link-no-deref.sh | 17 +++++++++++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/copy.c b/src/copy.c
index aedbd0dc3..7964a6eeb 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2569,7 +2569,7 @@ skip:
&& ! chown_failure_ok (x))
{
error (0, errno, _("failed to preserve ownership for %s"),
- dst_name);
+ quoteaf (dst_name));
if (x->require_preserve)
goto un_backup;
}
diff --git a/tests/cp/link-no-deref.sh b/tests/cp/link-no-deref.sh
index 92d4d36b5..c21764355 100755
--- a/tests/cp/link-no-deref.sh
+++ b/tests/cp/link-no-deref.sh
@@ -18,6 +18,8 @@
. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
print_ver_ cp
+getlimits_
+uses_strace_
ln -s no-such-file dangling-slink || framework_failure_
@@ -26,4 +28,19 @@ ln -s no-such-file dangling-slink || framework_failure_
# with link being applied to the dangling symlink.
cp --link --no-dereference dangling-slink d2 || fail=1
+NL='
+'
+chowns=chown,lchown,fchownat
+if strace --quiet=all -o /dev/null -P foo \
+ -e inject=$chowns:error=EPERM true 2>/dev/null; then
+ echo "cp: failed to preserve ownership for ''\$'\\n': $ENOSYS" >exp \
+ || framework_failure_
+ touch file && ln -s file slink || framework_failure_
+ returns_ 1 strace --quiet=all -o /dev/null -P "$NL" \
+ -e inject=$chowns:error=ENOSYS cp -P --preserve=ownership slink "$NL" \
+ >out 2>err || fail=1
+ compare exp err || fail=1
+ compare /dev/null out || fail=1
+fi
+
Exit $fail
The patch looks good as is.
I wouldn't have added the test just to check the quoting,
but there is no other check I could find for this particular error message,
so it's probably worth adding that check.
We do have various sc_error_... syntax checks for these,
but they're heuristics and don't handle multi-line error() calls robustly.
I temp hacked the source to put error()s on a single line,
before running `make syntax_check` and found these further quoting issues,
which the attached should address. In the patch I've added a general item
about improved quoting given all these similar improvements.
cheers,
Padraig
From 62083c17ecd8d206626d926fe47d93a05177e8be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Fri, 23 Oct 2026 13:48:10 +0100
Subject: [PATCH] all: quote more user originated strings
* src/csplit.c: Quote arbitrary strings for clarity/safety.
* src/dircolors.c: Likewise.
* src/env.c: Likewise.
* src/split.c: Likewise.
* src/uniq.c: Likewise.
* tests/env/env-S.pl: Adjust accordingly.
* NEWS: Mention the improvement.
---
NEWS | 2 ++
src/csplit.c | 2 +-
src/dircolors.c | 2 +-
src/env.c | 2 +-
src/split.c | 6 +++---
src/uniq.c | 2 +-
tests/env/env-S.pl | 9 +++++----
7 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/NEWS b/NEWS
index b315ce7f4..df72676d9 100644
--- a/NEWS
+++ b/NEWS
@@ -127,6 +127,8 @@ GNU coreutils NEWS -*- outline -*-
have stricter validation of the $GLIBC_TUNABLES environment variable
that is now more aligned with GLIBC's handling.
+ Various commands quote arbitrary strings in error messages for clarity.
+
** Build-related
gzip-compressed tarballs are no longer distributed,
diff --git a/src/csplit.c b/src/csplit.c
index 0c8e6a1c5..c5219379f 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -1087,7 +1087,7 @@ extract_regexp (int argnum, bool ignore, char const *str)
closing_delim = strrchr (str + 1, delim);
if (closing_delim == NULL)
error (EXIT_FAILURE, 0,
- _("%s: closing delimiter '%c' missing"), str, delim);
+ _("%s: closing delimiter '%c' missing"), quote (str), delim);
len = closing_delim - str - 1;
p = new_control_record ();
diff --git a/src/dircolors.c b/src/dircolors.c
index bd7706f05..7787b13a5 100644
--- a/src/dircolors.c
+++ b/src/dircolors.c
@@ -392,7 +392,7 @@ dc_parse_stream (FILE *fp, char const *filename)
{
error (0, 0, _("%s:%td: unrecognized keyword %s"),
(filename ? quotef (filename) : _("<internal>")),
- line_number, keywd);
+ line_number, quote (keywd));
ok = false;
}
diff --git a/src/env.c b/src/env.c
index 27bbf5e2a..b4e517c0c 100644
--- a/src/env.c
+++ b/src/env.c
@@ -736,7 +736,7 @@ build_argv (char const *str, int extra_argc, int *argc)
if (!n)
error (EXIT_CANCELED, 0,
_("only ${VARNAME} expansion is supported, error at: %s"),
- str);
+ quote (str));
char *v = getenv (n);
if (v)
diff --git a/src/split.c b/src/split.c
index 24f2a36ad..fd3dae24b 100644
--- a/src/split.c
+++ b/src/split.c
@@ -609,7 +609,7 @@ create (char const *name)
(char * const *) argv, environ);
if (result != 0)
error (EXIT_FAILURE, errno, _("failed to run command: \"%s -c %s\""),
- shell_prog, filter_command);
+ quotef (shell_prog), quoteaf_n (1, filter_command));
posix_spawnattr_destroy (&attr);
posix_spawn_file_actions_destroy (&actions);
@@ -665,7 +665,7 @@ closeout (FILE *fp, int fd, pid_t pid, char const *name)
sprintf (signame, "%d", sig);
error (sig + 128, 0,
_("with FILE=%s, signal %s from command: %s"),
- quotef (name), signame, filter_command);
+ quotef (name), signame, quoteaf_n (1, filter_command));
}
}
else if (WIFEXITED (wstatus))
@@ -673,7 +673,7 @@ closeout (FILE *fp, int fd, pid_t pid, char const *name)
int ex = WEXITSTATUS (wstatus);
if (ex != 0)
error (ex, 0, _("with FILE=%s, exit %d from command: %s"),
- quotef (name), ex, filter_command);
+ quotef (name), ex, quoteaf_n (1, filter_command));
}
else
{
diff --git a/src/uniq.c b/src/uniq.c
index f91c4212c..d60541428 100644
--- a/src/uniq.c
+++ b/src/uniq.c
@@ -248,7 +248,7 @@ size_opt (char const *opt, char const *msgid)
intmax_t size;
if (LONGINT_OVERFLOW < xstrtoimax (opt, NULL, 10, &size, "")
|| size < 0)
- error (EXIT_FAILURE, 0, "%s: %s", opt, _(msgid));
+ error (EXIT_FAILURE, 0, "%s: %s", quote (opt), _(msgid));
return MIN (size, IDX_MAX);
}
diff --git a/tests/env/env-S.pl b/tests/env/env-S.pl
index 7b0912ab7..18f421a4e 100755
--- a/tests/env/env-S.pl
+++ b/tests/env/env-S.pl
@@ -229,16 +229,17 @@ my @Tests =
['err5', q[-S'A=B\\q'], {EXIT=>125},
{ERR=>"$prog: invalid sequence '\\q' in -S\n"}],
['err6', q[-S'A=$B'], {EXIT=>125},
- {ERR=>"$prog: only \${VARNAME} expansion is supported, error at: \$B\n"}],
+ {ERR=>"$prog: only \${VARNAME} expansion is supported, " .
+ "error at: '\$B'\n"}],
['err7', q[-S'A=${B'], {EXIT=>125},
{ERR=>"$prog: only \${VARNAME} expansion is supported, " .
- "error at: \${B\n"}],
+ "error at: '\${B'\n"}],
['err8', q[-S'A=${B%B}'], {EXIT=>125},
{ERR=>"$prog: only \${VARNAME} expansion is supported, " .
- "error at: \${B%B}\n"}],
+ "error at: '\${B%B}'\n"}],
['err9', q[-S'A=${9B}'], {EXIT=>125},
{ERR=>"$prog: only \${VARNAME} expansion is supported, " .
- "error at: \${9B}\n"}],
+ "error at: '\${9B}'\n"}],
# Test incorrect shebang usage (extraneous whitespace).
['err_sp2', q['-v -S cat -n'], {EXIT=>125},
--
2.55.0
