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

[FR] gopass move to rename/move directories #892

Closed
tagirb opened this issue Jul 2, 2018 · 6 comments · Fixed by #2718
Closed

[FR] gopass move to rename/move directories #892

tagirb opened this issue Jul 2, 2018 · 6 comments · Fixed by #2718
Assignees
Labels
Milestone

Comments

@tagirb
Copy link

tagirb commented Jul 2, 2018

Summary

Currently gopass move only supports moving password entries around. Is it possible to also implement moving whole subpaths/directories around? For git backend this is as simple as git mv && git commit && git push, which is my workaround for now.

@dominikschulz dominikschulz added feature Enhancements and new features help-wanted labels Jul 16, 2018
@dominikschulz dominikschulz added this to the 1.x.x milestone Jul 16, 2018
@stale
Copy link

stale bot commented Oct 14, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 14, 2018
@espoelstra
Copy link

@dominikschulz apparently things went a slightly different direction at some point.

Today I did a gopass show SomeFolder/SomeSecret and having used gopass edit --create SomeFolder/SomeSecret/AnotherSecret I got a message telling me that I had both a secret and a folder at the path, so to use gopass list to view the folder contents, and gopass show to view the secret. This worked as expected, but trying a gopass mv SomeFolder/SomeSecret SomeFolder/NewSecretName, it instead moved BOTH the SomeSecret folder AND the SomeSecret entry under the new path. I would hope that it could be scoped to prefer only selecting the secret to be moved if both a secret and a directory exist. The worst part was I ended up with SomeSecret/NewSecretName/SomeSecret/AnotherSecret and SomeSecret/NewSecretName/SomeSecret, so I get to work through some duplication in the path as well.

@AnomalRoil
Copy link
Member

Confirmed to still be the case as described in the latest comment.

@dominikschulz dominikschulz modified the milestones: 1.12.0, 1.12.1 Jan 21, 2021
@AnomalRoil AnomalRoil modified the milestones: 1.12.1, 1.13.0 Sep 29, 2021
@dominikschulz dominikschulz modified the milestones: 1.13.0, 1.x.x Dec 4, 2022
@dominikschulz dominikschulz changed the title feature request: gopass move to rename/move directories [FR] gopass move to rename/move directories Dec 4, 2022
@leogott
Copy link

leogott commented Nov 16, 2023

So I've encountered the problem before, that after creating a secret some/example I created another secret some/example/account2.

This is not ideal, but also not a big deal, git mv, commit, push …
However when attempting to use gopass move some/example some/example/account1, it quietly moves the dir instead of the file, resulting in this:
some/example/account1/account2 // this is account2
some/example // this is account1

I think I've identified the code responsible and the necessary change, however I've not written in Go before.

func (r *Store) move(ctx context.Context, from, to string, del bool) error {
subFrom, fromPrefix := r.getStore(from)
subTo, _ := r.getStore(to)
srcIsDir := r.IsDir(ctx, from)
dstIsDir := r.IsDir(ctx, to)

// L45 becomes
srcIsDir := r.IsDir(ctx, from) && !r.Exists(ctx, from)  // move the dir iff no secret exists at `from`

// Insert this at L47
if r.IsDir(ctx, from) && r.Exists(ctx, from) {
    // Output some warning about the ambiguity and state that gopass is moving the secret
    // Maybe ask for confirmation
    // (Do _not_ ask if they instead want to move the dir, in my opinion)
}

if srcIsDir && r.Exists(ctx, to) && !dstIsDir {
return fmt.Errorf("destination is a file")
}
if err := r.moveFromTo(ctx, subFrom, from, to, fromPrefix, srcIsDir, dstIsDir, del); err != nil {
return err
}

@AnomalRoil
Copy link
Member

Thanks for the investigation, you almost had it, there was another problem slightly further down in moveFromTo, where we weren't using srcIsDir properly and so fixing it here didn't help.

But hopefully #2718 should now fix this (very) longstanding issue 😳 😅

AnomalRoil added a commit that referenced this issue Nov 23, 2023
@leogott
Copy link

leogott commented Nov 28, 2023

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants