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

secrets edit command race condtion #149

Closed
tegefaulkes opened this issue Mar 1, 2024 · 3 comments · Fixed by #193
Closed

secrets edit command race condtion #149

tegefaulkes opened this issue Mar 1, 2024 · 3 comments · Fixed by #193
Assignees
Labels
development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management

Comments

@tegefaulkes
Copy link
Contributor

Specification

The secrets edit command writes the secret's contents to a temp file and opens an editor. The problem is this will be the same file across all concurrent calls of secrets edit resulting in clobbering.

const tmpDir = `${os.tmpdir}/pksecret`;
await this.fs.promises.mkdir(tmpDir);
const tmpFile = `${tmpDir}/pkSecretFile`;
await this.fs.promises.writeFile(tmpFile, secretContent);
execSync(`$EDITOR \"${tmpFile}\"`, { stdio: 'inherit' });

To fix this we'll need to make a tmpdir similar to how our tests work.

    dataDir = await fs.promises.mkdtemp(
      path.join(os.tmpdir(), 'polykey-test-'),
    );

Additional context

Tasks

  1. Use a generated temp directory when creating the tempfile.
  2. Add a test to check for concurrent usage and clobbering.
@tegefaulkes tegefaulkes added the development Standard development label Mar 1, 2024
@tegefaulkes tegefaulkes self-assigned this Jun 4, 2024
@CryptoTotalWar
Copy link

There's also an issue with secrets edit command that i think might be specific to MAC where it says that the editor doesn't have the required permissions to perform a secrets edit command and that permissions need to be configured at the admin level to do so. something like that. Let me know if you need me to provide more context and screenshots for this @tegefaulkes

@CMCDragonkai
Copy link
Member

Yes create a new bug issue for that in Polykey-CLI, because Macs sometimes have specific permission issues. In this case, we are trying to create a temporary file and edit it. But wait until this is released. @tegefaulkes is this in the current release or next one?

Copy link
Contributor Author

@pablo.padillo You should test this again. If there is till an issue then make a bug issue for it.

@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management
Development

Successfully merging a pull request may close this issue.

3 participants