Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#43202 closed defect (fixed)

macports.tcl: doesn't reset user if svn fails

Reported by: mojca (Mojca Miklavec) Owned by: neverpanic (Clemens Lang)
Priority: Normal Milestone: MacPorts 2.3.5
Component: base Version: 2.2.99
Keywords: haspatch Cc:
Port:

Description

When svn fails to sync the ports (in my case that's because it doesn't accept the certificate), rsync continues to run as a local user. I'm attaching an example of a patch, but please check it carefully first as I don't have no experience messing up with base.

Maybe something similar is needed at other places as well.

(The patch is against trunk, but it would be nice to fix this for 2.3.0 as well.)

Attachments (1)

set_uid.patch (672 bytes) - added by mojca (Mojca Miklavec) 6 years ago.
a patch to set user back to root even when svn fails

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by mojca (Mojca Miklavec)

Attachment: set_uid.patch added

a patch to set user back to root even when svn fails

comment:1 Changed 6 years ago by mojca (Mojca Miklavec)

One way to test this it to add one extra debug line with geteuid to /opt/local/share/macports/Tcl/macports1.0/macports.tcl:

set needs_portindex 0
ui_info "Synchronizing local ports tree from $source"
ui_info ">>> Hello me ([geteuid])"

and to replace

system $svn_commandline

with

system "/usr/bin/false"

comment:2 Changed 6 years ago by neverpanic (Clemens Lang)

Owner: changed from macports-tickets@… to cal@…
Status: newassigned

Thanks, that also was a problem for git. r118570. Should we backport that into 2.3 aswell?

comment:3 Changed 6 years ago by neverpanic (Clemens Lang)

Cc: cal@… removed
Milestone: MacPorts Future
Resolution: fixed
Status: assignedclosed

comment:4 Changed 6 years ago by mojca (Mojca Miklavec)

Thank you. As already mentioned it would be nice to see it in 2.3.0, but that's up to you or others to decide.

I forgot to say earlier (and maybe I could write a patch as well), but it would probably be helpful to add a bit of extra ui_debug info. For example the following:

set euid [geteuid]
set egid [getegid]
ui_debug "changing euid/egid - current euid: $euid - current egid: $egid"
setegid [name_to_gid [file attributes $portdir -group]]
seteuid [name_to_uid [file attributes $portdir -owner]]

would be more helpful if both initial and final UID and GID were reported (ideally also with username, not just number?). And it would probably be nice to cleary report the the change during reversal as well.

An alternative would be to change all reports like

ui_debug $svn_commandline

into something like

ui_debug "Running '$svn_commandline' as [geteuid]"

but that would basically mean modifying debug all commands. Maybe that's too much.

What do you think of moving ui_debug $svn_commandline from before the loop to just before it gets executed, so that UID change would be reported before it is printed out?

comment:5 in reply to:  4 Changed 6 years ago by neverpanic (Clemens Lang)

Replying to mojca@…:

Thank you. As already mentioned it would be nice to see it in 2.3.0, but that's up to you or others to decide.

I'm not in any way more qualified to decide whether to backport that to 2.3.0 than you are. In fact, you could just go ahead and cherry-pick the change onto the 2.3 branch.

I forgot to say earlier (and maybe I could write a patch as well), but it would probably be helpful to add a bit of extra ui_debug info. For example the following:

set euid [geteuid]
set egid [getegid]
ui_debug "changing euid/egid - current euid: $euid - current egid: $egid"
setegid [name_to_gid [file attributes $portdir -group]]
seteuid [name_to_uid [file attributes $portdir -owner]]

would be more helpful if both initial and final UID and GID were reported (ideally also with username, not just number?). And it would probably be nice to cleary report the the change during reversal as well.

Actually I was surprised the change was done manually. There are a few other places in MacPorts where euid/egid are set and those are IIRC capsuled in a proc that also prints some debugging info. Maybe re-using that code and making it print nicer messages would be the way to go?

In fact, one could re-factor the whole thing as a block statement with appropriate error handling, like registry::write { code } does, something along the lines of

runAsUser $uid $gid {
  # run svn command here
}

that would set euid/egid, print debug messages, run the block of code with the changed privileges and restore the previous uid/gid when the block finishes or throws an error.

comment:6 Changed 4 years ago by raimue (Rainer Müller)

Milestone: MacPorts FutureMacPorts 2.3.5
Note: See TracTickets for help on using tickets.