Skip to content
This repository was archived by the owner on May 29, 2025. It is now read-only.

Conversation

@jmoreira-valory
Copy link
Collaborator

@jmoreira-valory jmoreira-valory commented Oct 18, 2023

Feature to protect keys generated by the script.

IMPORTANT: In order to be merged, this PR requires a version of the trader that supports a version of Open Autonomy including this fix: valory-xyz/open-autonomy#2108

@jmoreira-valory jmoreira-valory changed the base branch from main to feat/store_pkeys October 18, 2023 20:13
@jmoreira-valory jmoreira-valory changed the base branch from feat/store_pkeys to main October 19, 2023 11:46
@DavidMinarsch DavidMinarsch changed the base branch from main to develop November 17, 2023 16:43
@jmoreira-valory
Copy link
Collaborator Author

@jmoreira-valory jmoreira-valory mentioned this pull request Nov 20, 2023
Copy link
Collaborator

@Adamantios Adamantios left a comment

Choose a reason for hiding this comment

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

Tested this and the password is not enforced. I do not agree with releasing this as is.

@jmoreira-valory
Copy link
Collaborator Author

Tested this and the password is not enforced. I do not agree with releasing this as is.

Please clarify what enforcements and where.

@Adamantios
Copy link
Collaborator

Adamantios commented Nov 21, 2023

Tested this and the password is not enforced. I do not agree with releasing this as is.

Please clarify what enforcements and where.

I first used password test.
Then I decided to test changing the password. Message said it succeeded.
Then I ran the script again but tried the old password. It worked. Then I tried a totally irrelevant password, neither the old nor the new one. It worked. Instead, it should only proceed when the password is correct, but it seems that the password is never checked.

Copy link
Collaborator

@Adamantios Adamantios left a comment

Choose a reason for hiding this comment

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

Tested this again. The script just validates the password later and not immediately.

@valory-xyz valory-xyz deleted a comment from jmoreira-valory Nov 21, 2023
@jmoreira-valory
Copy link
Collaborator Author

Tested this again. The script just validates the password later and not immediately.

The script validates the password after downloading the trader repository.
It needs the Open Autonomy packages to try decrypt the passowrd and decide if it validates it or not.

Possible UX improvement:

  1. First download trader repo.
  2. Ask for password
  3. Validate password

(currently we have 2, 1, 3)

@DavidMinarsch DavidMinarsch merged commit cfb685b into develop Nov 21, 2023
@DavidMinarsch DavidMinarsch deleted the feat/script_security branch November 21, 2023 17:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants