Creating secrets env command

[tegefaulkes]

Description

This PR addresses adding the secrets env command to the cli

Issues Fixed

• Fixes The pk secrets env command for meeting Development Environment Usecase #31

Tasks

• 1. parse input for selecting desired secrets from vaults
• 2. obtain secrets from the Polykey agent
• 3. ability to output env variables in selectable formats
• 4. ability to run commands with env variables only for the duration of that command
• 5. ability to have the running command replace the current process that invoked it.
• 6. created tests to demonstrate and test functionality

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

Legend

[tegefaulkes]

There will be a companion PR in Polykey for tracking the lib changes needed. There will also likely be a new repo for the native code needed to do kexec.

[CMCDragonkai]

The repo https://github.com/MatrixAI/js-exec still needs some work - lots of configuration needed - README, and permissions, CI integration into Gitlab.

[tegefaulkes]

Yeah, I'm still in the middle of working on it.

[CMCDragonkai]

@tegefaulkes the execvpe does not work on MacOS.

For Unix compatibility you need to start with excevp, and simulate execvpe by setting the environment yourself.

Example provided in C:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char *argv[], char *envp[]) {
    // Add or modify an environment variable
    setenv("MY_ENV_VAR", "some_value", 1);

    // Prepare arguments for execvp
    char *new_argv[] = {"program_name", "arg1", "arg2", NULL};

    // Execute the program with the modified environment
    execvp(new_argv[0], new_argv);

    // If execvp returns, it failed
    perror("execvp");
    return EXIT_FAILURE;
}

However, since you're using rust, I'm sure there's equivalents for it.

What's the difference between using:

• https://docs.rs/uapi/latest/uapi/fn.execvpe.html
• https://docs.rs/libc/latest/libc/fn.execvpe.html

[CMCDragonkai]

Have a look: https://chat.openai.com/share/e/9c9f1315-d600-49c4-91a1-667ecb8ca0cb

Example with libc

extern crate libc;

use libc::{execvp, setenv};
use std::ffi::CString;
use std::os::unix::ffi::OsStrExt;
use std::process;

fn main() {
    // Set an environment variable
    let key = CString::new("MY_VARIABLE").unwrap();
    let value = CString::new("some_value").unwrap();
    unsafe {
        if setenv(key.as_ptr(), value.as_ptr(), 1) != 0 {
            eprintln!("Failed to set environment variable");
            process::exit(1);
        }
    }

    // Prepare the arguments for execvp
    let program = CString::new("/usr/bin/env").unwrap();
    let args = [
        CString::new("env").unwrap().as_ptr(), // argv[0], conventionally the program name
        std::ptr::null(),                       // Arguments must be terminated by a NULL
    ];

    // Execute the command, replacing the current process
    unsafe {
        execvp(program.as_ptr(), args.as_ptr());
        // If execvp returns, it means an error occurred
        eprintln!("Error executing execvp");
        process::exit(1);
    }
}

Also I see alot of vestigial comments, can you clean up the code of js-exec and make sure errors are all handled.

[CMCDragonkai]

If uapi is better, you can use that but you'll need to figure out how to set the environment. https://stackoverflow.com/questions/59922661/why-doesnt-unistd-h-execvpe-work-on-macos

[CMCDragonkai]

I updated a bit of the repo configuration in https://github.com/MatrixAI/js-exec. The usage in README.md should be updated too.

Note that since on Windows it is expected to throw an exception, can you create a src/errors.ts to create the domain set of errors.

Also on Windows, to simulate the same behaviour, you'd need to be able to create a subprocess and detach it and then terminate the parent process. This can be put for a future issue to address, for now a standard childprocess can be done on Windows. Which means for now using this may be a bit inefficient on Windows.

[tegefaulkes]

OK, so we need to add the following before we can

1. We need to validate secret and override names to make sure they're valid environment variable names.
2. add flags for switching the behaviour of the following; we need to switch between throw, warning or silent. default to throw.
   a. how to handle duplicate env names
   b. how to handle invalid env names
3. we need to support the windows platform. Just need to switch to an alternative such as child_process.exec do match the behaviour but without process replacement.
4. General polish and review.

I also need to go back to the js-exec and review that too. Make sure that all commentary is addressed.

[tegefaulkes]

TODO:

• 1. when renaming a secret we need to validate the new secret name
• 2. when getting secret name from the vault we need to validate if the secret name is valid
• 3. when validating the secret name we need to switch between 3 behaviours. 'ignore', 'warn', 'error'.
• 4. when handling duplicate secret names we need to switch between the following behaviour. keep, overwrite, warn, error.
• 5. when running on windows we need to switch to using child_process.exec to run the command. This means we can only dynamically load js-exec if it's supported by the platform.
• 6. general polish
• 7. general polish of js-exec lib.

[tegefaulkes]

One thing I'm concerned about is that I'm using two different format options.

1. --format is not actually used, is included by default in the command and I cant actually override it.
2. --env-format is an option I've created with the following options. 'dotenv', 'json', 'prepend'.

Ideally I'd just have --format function like --env-format but we'd have to remove --format from being automatically included in all commands and re add it in to all command implementations.

[tegefaulkes]

I'ts possible to modify the options that have already been set. within the constructor when creating the command we can access the options array with this.options. It's in the format of Array<Option>. We can add or remove choices to the this.options[x].argChoices.push('newOption') so we just need to find the option we want to modify.

Note that while this.options can be accessed its not actually in the types.

[tegefaulkes]

Re-based on staging and ready to merge.

[CMCDragonkai]

One thing I'm concerned about is that I'm using two different format options.

1. --format is not actually used, is included by default in the command and I cant actually override it.
2. --env-format is an option I've created with the following options. 'dotenv', 'json', 'prepend'.

Ideally I'd just have --format function like --env-format but we'd have to remove --format from being automatically included in all commands and re add it in to all command implementations.

Hold up! Did you have a read about #31 (comment) - there's important differences between different terminal/shell environments.

And also it's important to acknowledge what we mean by the --format option. It's a program wide option to specify what is the format of data we should provide via STDOUT and STDERR. The default of course is --format human which is supposed to mean that it is "readable" by human beings.

Now if you're going to overload that option I need you to explain what prepend or dotenv is supposed to be. Plus you must document it in the CLI help too.

Here's what I'm proposing.

• human means human-readable - it doesn't get any more specific that that

The problem is that we may deal 3 different "shell" environments:

• Unix shells where X=Y works
• CMD
• Powershell

It is important to note that none of these variables should be auto exported without an additional flag being --export option.

So... I think human should automatically choose the right format by checking the parent process shell some how.

IF we cannot do that, then if you overload the --format then you need to decide what human means, and what if you can instead do unix, cmd, powershell formats instead.

[CMCDragonkai]

So by default variables are only local to the shell. Then with an extra --export flag, depending on the type of the shell that is running, you would then wrap the whole thing an export system. For unix, you'd use export X=Y, for powershell you need $env:X = "", for CMD there's a different method again.

[CMCDragonkai]

You might want to address the --export option in a separate issue.

[CMCDragonkai]

Make sure to test this on the windows and mac machines we got too!

[tegefaulkes]

Added follow on issues at #145 and #144