code review 60480045: cmd/go: do not create world-writable files.

cmd/go: do not create world-writable files.
Fixes issue 7240.

[minux1, 2014-02-06 06:44:48]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[Tv, 2014-02-06 20:15:08]

On 2014/02/06 06:44:48, minux wrote:
> Hello mailto:golang-codereviews@googlegroups.com,
> 
> I'd like you to review this change to
> https://code.google.com/p/go

This seems like it'll still be wrong for a setup where users share a primary
group: now jdoe:users with umask 0022 will be creating a g+w file.

[minux1, 2014-02-06 20:33:34]

On Thu, Feb 6, 2014 at 3:15 PM, <tommi.virtanen@gmail.com> wrote:

> On 2014/02/06 06:44:48, minux wrote:
>
>> Hello mailto:golang-codereviews@googlegroups.com,
>>
>
>  I'd like you to review this change to
>> https://code.google.com/p/go
>>
>
> This seems like it'll still be wrong for a setup where users share a
> primary group: now jdoe:users with umask 0022 will be creating a g+w
> file.

ok, g-w. PTAL.

[Tv, 2014-02-06 20:43:41]

On 2014/02/06 20:33:34, minux wrote:
> On Thu, Feb 6, 2014 at 3:15 PM, <mailto:tommi.virtanen@gmail.com> wrote:
> 
> > On 2014/02/06 06:44:48, minux wrote:
> >
> >> Hello mailto:golang-codereviews@googlegroups.com,
> >>
> >
> >  I'd like you to review this change to
> >> https://code.google.com/p/go
> >>
> >
> > This seems like it'll still be wrong for a setup where users share a
> > primary group: now jdoe:users with umask 0022 will be creating a g+w
> > file.
> 
> ok, g-w. PTAL.

I'm quite pessimistic about just blindly using 0755 leading to anything good,
either. I can't see a reliable way out of this without having the end result
affected by umask somehow. If you can't / don't want to deal with umask, you
can't chmod blindly. Perhaps you should take a isExec bool and do a
read-edit-write cycle on the mode, adjusting just that?

[Tv, 2014-02-06 20:47:57]

On 2014/02/06 20:43:41, Tv wrote:
> On 2014/02/06 20:33:34, minux wrote:
> > On Thu, Feb 6, 2014 at 3:15 PM, <mailto:tommi.virtanen@gmail.com> wrote:
> > 
> > > On 2014/02/06 06:44:48, minux wrote:
> > >
> > >> Hello mailto:golang-codereviews@googlegroups.com,
> > >>
> > >
> > >  I'd like you to review this change to
> > >> https://code.google.com/p/go
> > >>
> > >
> > > This seems like it'll still be wrong for a setup where users share a
> > > primary group: now jdoe:users with umask 0022 will be creating a g+w
> > > file.
> > 
> > ok, g-w. PTAL.
> 
> I'm quite pessimistic about just blindly using 0755 leading to anything good,
> either. I can't see a reliable way out of this without having the end result
> affected by umask somehow. If you can't / don't want to deal with umask, you
> can't chmod blindly. Perhaps you should take a isExec bool and do a
> read-edit-write cycle on the mode, adjusting just that?

On second thought, there's probably a bunch of other things doing install -m0755
and equivalent, so I guess 0755 / 0644 have to be good enough. (If your umask is
weirder than that, things just in general won't follow suit.)

LGTM

[bradfitz, 2014-02-06 22:32:17]

LGTM



On Wed, Feb 5, 2014 at 10:44 PM, <minux.ma@gmail.com> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> cmd/go: do not create world-writable files.
> Fixes issue 7240.
>
> Please review this at https://codereview.appspot.com/60480045/
>
> Affected files (+6, -6 lines):
>   M src/cmd/go/build.go
>
>
> Index: src/cmd/go/build.go
> ===================================================================
> --- a/src/cmd/go/build.go
> +++ b/src/cmd/go/build.go
> @@ -816,7 +816,7 @@
>                                 continue
>                         }
>                         coverFile := filepath.Join(obj, file)
> -                       if err := b.cover(a, coverFile, sourceFile, 0666,
> cover.Var); err != nil {
> +                       if err := b.cover(a, coverFile, sourceFile, 0664,
> cover.Var); err != nil {
>                                 return err
>                         }
>                         gofiles = append(gofiles, coverFile)
> @@ -911,17 +911,17 @@
>                 switch {
>                 case strings.HasSuffix(name, _goos_goarch):
>                         targ := file[:len(name)-len(_goos_goarch)] +
> "_GOOS_GOARCH." + ext
> -                       if err := b.copyFile(a, obj+targ,
> filepath.Join(a.p.Dir, file), 0666); err != nil {
> +                       if err := b.copyFile(a, obj+targ,
> filepath.Join(a.p.Dir, file), 0664); err != nil {
>                                 return err
>                         }
>                 case strings.HasSuffix(name, _goarch):
>                         targ := file[:len(name)-len(_goarch)] + "_GOARCH."
> + ext
> -                       if err := b.copyFile(a, obj+targ,
> filepath.Join(a.p.Dir, file), 0666); err != nil {
> +                       if err := b.copyFile(a, obj+targ,
> filepath.Join(a.p.Dir, file), 0664); err != nil {
>                                 return err
>                         }
>                 case strings.HasSuffix(name, _goos):
>                         targ := file[:len(name)-len(_goos)] + "_GOOS." +
> ext
> -                       if err := b.copyFile(a, obj+targ,
> filepath.Join(a.p.Dir, file), 0666); err != nil {
> +                       if err := b.copyFile(a, obj+targ,
> filepath.Join(a.p.Dir, file), 0664); err != nil {
>                                 return err
>                         }
>                 }
> @@ -993,9 +993,9 @@
>                 }
>         }()
>         a1 := a.deps[0]
> -       perm := os.FileMode(0666)
> +       perm := os.FileMode(0664)
>         if a1.link {
> -               perm = 0777
> +               perm = 0775
>         }
>
>         // make target directory
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>

[minux1, 2014-02-06 22:54:37]

*** Submitted as https://code.google.com/p/go/source/detail?r=665476aeaf5c ***

cmd/go: do not create world-writable files.
Fixes issue 7240.

LGTM=tommi.virtanen, bradfitz
R=golang-codereviews, tommi.virtanen, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/60480045