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

freak12techno · 2024-04-17T00:00:04Z

Description

Closes: #20061

Sets file permissions for upgrade directory and the binary itself to 0755 (owner can do everything, others can read and execute) when adding an upgrade through the cosmovisor add-upgrade command.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

included the correct type prefix in the PR title
confirmed ! in the type prefix if API or client breaking change
targeted the correct branch (see PR Targeting)
provided a link to the relevant issue or specification
reviewed "Files changed" and left comments if necessary
included the necessary unit and integration tests
added a changelog entry to CHANGELOG.md
updated the relevant documentation or specification, including comments for documenting Go code
confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

confirmed the correct type prefix in the PR title
confirmed all author checklist items have been addressed
reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

Refactor
- Enhanced file and directory permission settings for improved functionality.
- Updated permissions for adding upgrades in the cosmovisor module.
- Fixed file permission settings to ensure proper functionality.
- Added a fix to address cosmovisor add-upgrade permissions.
- Documented permission fix in the CHANGELOG.

freak12techno · 2024-04-17T00:00:52Z

@julienrbrt can you review?
I set the permissions to 0755 as this is what I personally use, let me know if it's better to use other permissions.

coderabbitai · 2024-04-17T00:01:08Z

Summary of changes

tools/cosmovisor/cmd/cosmovisor/add_upgrade.go: In the AddUpgrade function, the file permission for creating directories and writing files has been changed from 0o750 to 0o755 and from 0o600 to 0o755. A change was made in the cosmovisor module to fix the permissions for adding upgrades.
tools/cosmovisor/CHANGELOG.md: Added a fix for cosmovisor add-upgrade permissions in pull request #20062.

Objectives from linked issues for validation

#20061: [Bug]: Cosmovisor's add-upgrade creates a binary that's not executable

Title

[Bug]: Cosmovisor's add-upgrade creates a binary that's not executable

Description

Is there an existing issue for this?

I have searched the existing issues

What happened?

Better show on example:
validator@pion-testnet ~/neutron #v3.0.4 ❯ cosmovisor add-upgrade v3.0.4 ~/go/bin/neutrond
9:04PM INF Using /home/validator/go/bin/neutrond for v3.0.4 upgrade module=cosmovisor
9:04PM INF Upgrade binary located at /home/validator/.neutrond/cosmovisor/upgrades/v3.0.4/bin/neutrond module=cosmovisor
validator@pion-testnet ~/neutron #v3.0.4 ❯ /home/validator/.neutrond/cosmovisor/upgrades/v3.0.4/bin/neutrond version
zsh: permission denied: /home/validator/.neutrond/cosmovisor/upgrades/v3.0.4/bin/neutrond

I guess it's because of this line

cosmos-sdk/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go

Line 113 in 2301e5e

if err := os.WriteFile(path, data, 0o600); err != nil {

that assigns the permissions as 600 (owner can read and write but not execute, others cannot read and write) and it should be changed to something more relaxed (not sure yet what).

Feel free to assign me to this issue if you want btw.

Cosmos SDK Version

cosmovisor version: v1.5.0

How to reproduce?

See above.

Instructions

Generate the below sections using H2 header for each:

Walkthrough: A high-level summary of the overall change instead of specific files within 80 words.
Changes: A markdown table of files and their change summaries. Group files with similar changes together into a single row to save space. Shorten long paths by using ellipsis for skippable parts.
Assessment against linked issues: Validate whether the code changes meet the primary objectives and requirements in the linked issues. The integrity of the codebase hinges on the thoroughness of your scrutiny. Focus on requirements related to coding and disregarding non-coding tasks such as manual testing instructions. Instructions for this section:
- This section must be a table with 3 columns: Objective, Addressed and Explanation.
- Group similar objectives together and provide issue numbers in the brackets.
- Use the following symbols in the 'Addressed' column:
  - ✅: if the objective has been addressed in the PR. The 'Explanation' column must be left blank.
  - ❌: if the objective has not been addressed in the PR. Provide a brief explanation in the 'Explanation' column.
  - ❓: if it is unclear whether the objective has been addressed. Provide a brief explanation in the 'Explanation' column.
  - Limit the assessment to the objectives from the linked issues only. Do not include objectives that do not exist.
  - Limit the table to less than 5 rows.
  - In the 'Explanation' column, do not provide change summary and be very specific and to the point.

Walkthrough

The recent changes in the AddUpgrade function of Cosmovisor address the issue of incorrect file permissions that resulted in non-executable binaries being created. By adjusting the permissions to more permissive settings (0o755), the upgrade binaries can now be executed as intended.

Changes

File Path	Change Summary
`tools/cosmovisor/cmd/cosmovisor/add_upgrade.go`	Changed file permissions for directories from `0o750` to `0o755` and for files from `0o600` to `0o755`.
`tools/cosmovisor/CHANGELOG.md`	Added a fix for cosmovisor add-upgrade permissions in pull request #20062.

Assessment against linked issues

Objective	Addressed	Explanation
#20061: Ensure executability of binaries created by `add-upgrade`	✅
#20061: Correct file permissions for upgrade binaries	✅

The changes made in the AddUpgrade function align with the objectives outlined in issue #20061 by ensuring that the upgrade binaries are created with the correct permissions for executability, resolving the reported issue effectively.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

tools/cosmovisor/cmd/cosmovisor/add_upgrade.go

freak12techno · 2024-04-17T00:10:58Z

lol I guess I know why it used these values initially

julienrbrt

thanks!

julienrbrt · 2024-04-17T06:46:08Z

tools/cosmovisor/cmd/cosmovisor/add_upgrade.go

@@ -110,7 +110,7 @@ func saveOrAbort(path string, data []byte, force bool) error {
 		return fmt.Errorf("failed to check if file exists: %w", err)
 	}

-	if err := os.WriteFile(path, data, 0o600); err != nil {
+	if err := os.WriteFile(path, data, 0o755); err != nil {


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?

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?

nevermind, I fixed it

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

CHANGELOG.md

coderabbitai

Actionable comments posted: 3

coderabbitai · 2024-04-17T15:18:27Z

tools/cosmovisor/CHANGELOG.md

@@ -35,6 +35,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 # Changelog

 ## [Unreleased]
+* [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions


Ensure the changelog entry is surrounded by blank lines for better readability and adherence to the changelog guidelines.

37a38 > 39a40 >

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

* [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions

* [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions

The changelog entry for PR #20062 could be more descriptive. Consider specifying what was fixed and why it's important.

- * [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions + * [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions to allow executable access, resolving a critical issue where upgrades were failing due to permission errors.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

* [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions

* [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions to allow executable access, resolving a critical issue where upgrades were failing due to permission errors.

coderabbitai · 2024-04-17T15:18:27Z

tools/cosmovisor/CHANGELOG.md

@@ -35,6 +35,8 @@
 # Changelog

 ## [Unreleased]
+* [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions
+



Multiple consecutive blank lines detected. It's best to maintain a single blank line to keep the file clean and organized.

40d39

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

freak12techno · 2024-04-17T23:13:58Z

Btw @julienrbrt once I'm on it. I may have found another really minor but somewhat annoying bug in logging I want to fix before 1.6.0:

cosmos-sdk/tools/cosmovisor/cmd/cosmovisor/add_upgrade.go

Line 97 in e81cdd5

    
           logger.Info(fmt.Sprintf("%s created, %s upgrade binary will switch at height %d", filepath.Join(cfg.UpgradeInfoFilePath(), upgradetypes.UpgradeInfoFilename), upgradeName, upgradeHeight))

cfg.UpgradeInfoFilePath() already includes the filename, so joining it would result in an invalid path (like blablabla/upgrade-info.json/upgrade-info.json), pretty sure it's not intended. This is not related to the issue I am fixing in this PR, but seems small enough to maybe fixing it here instead of making a separate PR and issue for it.
I can fix it there in this PR, as I'm touching this file already here, or make another issue and PR and fix it there, what do you think would be better?

julienrbrt · 2024-04-18T07:12:11Z

Fixing it here is fine! Thank you 🙏

freak12techno · 2024-04-18T09:38:00Z

@julienrbrt can you check once more?

julienrbrt

ACK

freak12techno · 2024-04-23T15:03:02Z

@julienrbrt anything else I should do to get it merged or is it all good?

julienrbrt · 2024-04-23T16:15:28Z

It's all good. Just waiting for another reviewer. Cc @facundomedica

fix: fixed cosmovisor add-upgrade permissions

9751b9d

freak12techno requested a review from a team as a code owner April 17, 2024 00:00

github-actions bot added the C:Cosmovisor Issues and PR related to Cosmovisor label Apr 17, 2024

github-advanced-security bot found potential problems Apr 17, 2024

View reviewed changes

tools/cosmovisor/cmd/cosmovisor/add_upgrade.go Dismissed Show dismissed Hide dismissed

tools/cosmovisor/cmd/cosmovisor/add_upgrade.go Dismissed Show dismissed Hide dismissed

julienrbrt changed the title ~~fix: fixed cosmovisor add-upgrade permissions~~ fix(cosmovisor): fixed cosmovisor add-upgrade permissions Apr 17, 2024

julienrbrt reviewed Apr 17, 2024

View reviewed changes

julienrbrt mentioned this pull request Apr 17, 2024

Prepare Cosmovisor 1.6.0 #20067

Open

7 tasks

freak12techno added 2 commits April 17, 2024 17:38

chore: fixed permissions

d119102

chore: add changelog entry

f29a766

julienrbrt reviewed Apr 17, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

chore: moved changelog entry

e81cdd5

freak12techno requested a review from julienrbrt April 17, 2024 15:14

julienrbrt approved these changes Apr 17, 2024

View reviewed changes

julienrbrt enabled auto-merge April 17, 2024 15:16

julienrbrt requested a review from tac0turtle April 17, 2024 15:16

coderabbitai bot reviewed Apr 17, 2024

View reviewed changes

julienrbrt disabled auto-merge April 18, 2024 07:12

freak12techno and others added 2 commits April 18, 2024 12:37

chore: fixed logging upgrade-info path

905649b

Merge branch 'main' into fix-cosmovisor-add-upgrade-permissions

affe403

julienrbrt approved these changes Apr 18, 2024

View reviewed changes

tac0turtle approved these changes May 2, 2024

View reviewed changes

tac0turtle added this pull request to the merge queue May 2, 2024

Merged via the queue into cosmos:main with commit 8f10576 May 2, 2024
55 of 60 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

freak12techno commented Apr 17, 2024 •

edited by coderabbitai bot

freak12techno commented Apr 17, 2024

coderabbitai bot commented Apr 17, 2024 •

edited

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

freak12techno commented Apr 17, 2024

julienrbrt left a comment

julienrbrt Apr 17, 2024

freak12techno Apr 17, 2024

freak12techno Apr 17, 2024

freak12techno Apr 17, 2024

coderabbitai bot left a comment

coderabbitai bot Apr 17, 2024

coderabbitai bot Apr 17, 2024

freak12techno commented Apr 17, 2024

julienrbrt commented Apr 18, 2024

freak12techno commented Apr 18, 2024

julienrbrt left a comment

freak12techno commented Apr 23, 2024

julienrbrt commented Apr 23, 2024

	* [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions
	* [#20062](https://github.com/cosmos/cosmos-sdk/pull/20062) Fixed cosmovisor add-upgrade permissions

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

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

Conversation

freak12techno commented Apr 17, 2024 • edited by coderabbitai bot

Description

Author Checklist

Reviewers Checklist

Summary by CodeRabbit

freak12techno commented Apr 17, 2024

coderabbitai bot commented Apr 17, 2024 • edited

Summary of changes

Objectives from linked issues for validation

#20061: [Bug]: Cosmovisor's add-upgrade creates a binary that's not executable

Title

Description

Is there an existing issue for this?

What happened?

Cosmos SDK Version

How to reproduce?

Instructions

Walkthrough

Changes

Assessment against linked issues

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

freak12techno commented Apr 17, 2024

julienrbrt left a comment

Choose a reason for hiding this comment

julienrbrt Apr 17, 2024

Choose a reason for hiding this comment

freak12techno Apr 17, 2024

Choose a reason for hiding this comment

freak12techno Apr 17, 2024

Choose a reason for hiding this comment

freak12techno Apr 17, 2024

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Apr 17, 2024

Choose a reason for hiding this comment

coderabbitai bot Apr 17, 2024

Choose a reason for hiding this comment

freak12techno commented Apr 17, 2024

julienrbrt commented Apr 18, 2024

freak12techno commented Apr 18, 2024

julienrbrt left a comment

Choose a reason for hiding this comment

freak12techno commented Apr 23, 2024

julienrbrt commented Apr 23, 2024

freak12techno commented Apr 17, 2024 •

edited by coderabbitai bot

coderabbitai bot commented Apr 17, 2024 •

edited

CodeRabbit Configration File (`.coderabbit.yaml`)