qri-io / qri

you're invited to a data party!
https://qri.io
GNU General Public License v3.0
1.11k stars 66 forks source link

`qri init` sets executable bit on the files it creates #1155

Closed feep closed 4 years ago

feep commented 4 years ago

Wrong permissions?

❯ qri init
Name of new dataset [scratch]:   
Format of dataset, csv or json [csv]: 
initialized working directory for new dataset feep/scratch

❯ ls 
.rw-r--r-- 12 rusty 03-05 14:25 .qri-ref
.rwxr-xr-x 22 rusty 03-05 14:25 body.csv
.rwxr-xr-x 18 rusty 03-05 14:25 meta.json
.rwxr-xr-x 36 rusty 03-05 14:25 structure.json
Hackerpilot commented 4 years ago

Looks like this is caused by the os.ModePerm constant being used when the files are created. I'm not really sure why this is, as I think this constant is supposed to be a bit-mask used to separate the permission bits from other parts of a file mode. The line that causes this specific issue seems to be https://github.com/qri-io/qri/blob/924157c253bf571ec9df92b0fd63e316e67bc6ad/base/component/kinds.go#L502

dustmop commented 4 years ago

@Hackerpilot your diagnosis sounds correct, that should be a relatively easy fix. I think it should be 0644 instead of the current value. It would be great to have a test updated to check for this behavior, for example TestInitStatusSave in the file cmd/fsi_integration_test.go is listing the directory and checking the filenames; it could also check the permissions on those files.

mecm1993 commented 4 years ago

This verification was added into TestInitStatusSave to validate the permission of each file,

// Verify the permissions for each generated file.
files := filesDirectory(workDir)
mode := int(0644)
expectPermission := os.FileMode(mode)
for _, file := range files {
        if file.Mode() != expectPermission {
        t.Errorf("%s does not have the correct permission", file.Name())
    }
}
...
func filesDirectory(path string) []os.FileInfo {
    finfos, err := ioutil.ReadDir(path)
    if err != nil {
        return nil
    }
    return finfos
}

Moreover, the following modifications can be added to kinds.go, so the correct permission can be assigned for the file.

const Perm = os.FileMode(int(0644))
...
func (bc *BodyComponent) WriteTo(dirPath string) (targetFile string, err error) {
    if bc.Value == nil {
        err = bc.LoadAndFill(nil)
        if err != nil {
            return
        }
    }
    body := bc.Value
    if bc.Structure == nil {
        return "", fmt.Errorf("cannot write body without a structure")
    }
    data, err := SerializeBody(body, bc.Structure)
    if err != nil {
        return "", err
    }
    bodyFilename := fmt.Sprintf("body.%s", bc.Format)
    targetFile = filepath.Join(dirPath, bodyFilename)
    return targetFile, ioutil.WriteFile(targetFile, data, Perm)
}
...
func writeComponentFile(value interface{}, dirPath string, basefile string) (string, error) {
    data, err := json.MarshalIndent(value, "", " ")
    if err != nil {
        return "", err
    }
    // TODO(dlong): How does this relate to Base.SourceFile? Should respect that.
    targetFile := filepath.Join(dirPath, basefile)
    err = ioutil.WriteFile(targetFile, data, Perm)
    if err != nil {
        return "", err
    }
    return targetFile, nil
}

There are two other files readme and transform which are probably being generated with the wrong permissions due to the os.ModePerm.

If the above points are good, I can open a PR with the changes.

dustmop commented 4 years ago

This has been fixed, thanks to @mecm1993