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.

Sign up for GitHub

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

Jump to bottom

fix(cosmovisor): fixed cosmovisor add-upgrade permissions #20062

Merged
merged 6 commits into from
May 2, 2024
Merged

fix(cosmovisor): fixed cosmovisor add-upgrade permissions #20062

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9751b9d
fix: fixed cosmovisor add-upgrade permissions
freak12techno Apr 16, 2024
d119102
chore: fixed permissions
freak12techno Apr 17, 2024
f29a766
chore: add changelog entry
freak12techno Apr 17, 2024
e81cdd5
chore: moved changelog entry
freak12techno Apr 17, 2024
905649b
chore: fixed logging upgrade-info path
freak12techno Apr 18, 2024
affe403
Merge branch 'main' into fix-cosmovisor-add-upgrade-permissions
freak12techno Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -117,6 +117,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* (x/bank) [#20028](https://github.com/cosmos/cosmos-sdk/pull/20028) Align query with multi denoms for send-enabled.
* (cli) [#20020](https://github.com/cosmos/cosmos-sdk/pull/20020) Make bootstrap-state command support both new and legacy genesis format.
* (baseapp) [#19616](https://github.com/cosmos/cosmos-sdk/pull/19616) Don't share gas meter in tx execution.
* (cosmovisor) [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions
freak12techno marked this conversation as resolved.
Show resolved Hide resolved

### API Breaking Changes

Expand Down
5 changes: 3 additions & 2 deletions tools/cosmovisor/cmd/cosmovisor/add_upgrade.go
View file
Open in desktop
Expand Up @@ -54,7 +54,7 @@

// create upgrade dir
upgradeLocation := cfg.UpgradeDir(upgradeName)
if err := os.MkdirAll(path.Join(upgradeLocation, "bin"), 0o750); err != nil {
if err := os.MkdirAll(path.Join(upgradeLocation, "bin"), 0o755); err != nil {
Dismissed Show dismissed Hide dismissed
return fmt.Errorf("failed to create upgrade directory: %w", err)
}

Expand Down Expand Up @@ -110,7 +110,8 @@
return fmt.Errorf("failed to check if file exists: %w", err)
}

if err := os.WriteFile(path, data, 0o600); err != nil {
//nolint:gosec // We need broader permissions to make it executable
if err := os.WriteFile(path, data, 0o755); err != nil {
Dismissed Show dismissed Hide dismissed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a changelog in cosmovisor and a nolint comment so we don't revert it back because of the gosec? And so that linting passes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

cmd/cosmovisor/add_upgrade.go:58:2: directive `//nolint:gosec` is unused for linter "gosec" (nolintlint)
	//nolint:gosec
	^
cmd/cosmovisor/add_upgrade.go:115:2: directive `//nolint:gosec` should provide explanation such as `//nolint:gosec // this is why` (nolintlint)
	//nolint:gosec
	^

lol now it gives me this. do you think it's worth disabling? or what's better to do here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind, I fixed it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and also added the changelog entry. @julienrbrt can you check?

return fmt.Errorf("failed to write binary to location: %w", err)
}

Expand Down