Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove vendor.conf as it is no longer used #1539

Merged
merged 1 commit into from
Mar 18, 2020

Conversation

thaJeztah
Copy link
Contributor

@HuKeping
Copy link
Contributor

HuKeping commented Mar 13, 2020

/go/src/github.com/theupdateframework/notary/cmd/notary/keys.go,525,"Deferring unsafe method ""*os.File"" on type ""Close""",MEDIUM,HIGH,defer from.Close(),CWE-
/go/src/github.com/theupdateframework/notary/cmd/notary/keys.go,554,"Deferring unsafe method ""*os.File"" on type ""Close""",MEDIUM,HIGH,defer f.Close(),CWE-
/go/src/github.com/theupdateframework/notary/cmd/notary/util.go,50,Expect WriteFile permissions to be 0600 or less,MEDIUM,HIGH,"ioutil.WriteFile(t.output, payload, 064

Shall we use a defer function to close the os.File?

    from, err := os.Open(file)
    if err != nil {
        return err
    }
    defer from.Close()              // this seems not a good practice and gosec shouting.

wondering why this PR trigger this warning

@thaJeztah
Copy link
Contributor Author

Oh! It's the same as #1537 (comment). The version of the gosec linter is not pinned, and they recently did a new release.

I wanted to keep this change separate as it seemed more clean, but perhaps I should do a quick carry of that first commit

@thaJeztah
Copy link
Contributor Author

Opened #1540 with a carry. Will rebase this one after that

@HuKeping
Copy link
Contributor

Thanks @thaJeztah !

LGTM!

@HuKeping HuKeping merged commit c8a9108 into notaryproject:master Mar 18, 2020
@thaJeztah thaJeztah deleted the remove_vndr branch March 18, 2020 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants