git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] gitk: add user-defined custom commands

From
TWTim Wiederhake <twied@gmx.net>
Date
Aug 10, 2026, 19:28 UTC
Message-ID
<f3b73531581a6f94410d863339c5683ae8d63e0d.camel@gmx.net>
In-Reply-To
<xmqq7bm1d1au.fsf@gitster.g>
On Fri, 2026-08-07 at 15:16 -0700, Junio C Hamano wrote:
Show 57 quoted lines
> "Tim Wiederhake via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > +    set len [string length $cmd_template]
> > +    for {set i 0} {$i < $len} {incr i} {
> > +        if {[string index $cmd_template $i] eq "%" && $i + 1 <
> > $len} {
> > +            set next [string index $cmd_template [expr {$i + 1}]]
> > +            if {!$blame_computed && ($next eq "b" || $next eq
> > "l")} {
> > +                set blame [get_blame_origin]
> > +                set blame_id [lindex $blame 0]
> > +                set blame_line [lindex $blame 1]
> > +                set blame_computed 1
> > +            }
> > +            switch -- $next {
> > +                "%" { append cmd "%" }
> > +                "i" { append cmd $id }
> > +                "t" { append cmd [lindex $commitinfo($id) 0] }
> > +                "a" { append cmd [lindex $commitinfo($id) 1] }
> > +                "d" { append cmd [lindex $commitinfo($id) 2] }
> > +                "c" { append cmd [lindex $commitinfo($id) 3] }
> > +                "D" { append cmd [lindex $commitinfo($id) 4] }
> > +                "m" { append cmd [lindex $commitinfo($id) 5] }
> > +                "M" { if {[info exists markedid]} { append cmd
> > $markedid } }
> > +                "b" { append cmd $blame_id }
> > +                "f" { append cmd [get_diff_file] }
> > +                "l" { append cmd $blame_line }
> > +                default { append cmd "%" $next }
> > +            }
> > +            incr i
> > +        } else {
> > +            append cmd [string index $cmd_template $i]
> > +        }
> > +    }
> > +
> > +    if {[catch {exec sh -c $cmd 2>@1} output]} {
> 
> What do various members of $commitinfo field have?  I presume that
> title and message are pretty much free text under control of anybody
> who can write to the repository and entice you to run this command,
> so running with "sh -c $cmd" would require $cmd to be quoting the
> payload properly, or you'd be opening yourself to be an arbitrary
> command execution, no?  With template "echo '%t'" you thought you
> are just printing the title but if the title has "title?'; echo no'"
> in
> it, wouldn't cmd end up being 
> 
> 	echo 'title?'; echo no''
> 
> and a more creative type can use something other than "echo no", to
> have a process run under your name and do more interesting things,
> right?
> 
> Note that I no longer speak Tcl (even though I admit I used to), so
> if there is some "magic" that makes use of $cmd in {exec sh -c $cmd}
> safe, the above may be missing the mark by a mile.

You are right, the code is vulnerable to shell injection. The first version was calling the command directly, so no escaping was necessary.

I added the "sh -c" to facilitate process forking ("&") and simple exit code manipulation ("command && exit 42"). But by now I honestly am not sure anymore on what the best solution is: calling the command directly and have the user write a wrapper script if necessary; or add code to properly escape all data read from commits and retain the convenience.

Opinions?
Tim
Previous: Johannes Sixt
Message 9 of 9 in “gitk: add user-defined custom commands”
  1. gitk: add user-defined custom commandsTim Wiederhake via GitGitGadget, Aug 4, 2026
  2. Johannes SixtAug 5, 2026
  3. Tim WiederhakeAug 7, 2026
  4. Johannes SixtAug 9, 2026
  5. Tim WiederhakeAug 10, 2026
  6. Johannes SixtAug 11, 2026
  7. Junio C HamanoAug 7, 2026
  8. Johannes SixtAug 9, 2026
  9. Tim WiederhakeAug 10, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.