address pr comments

keps/sig-cli/kustomize-exec-secret-generator.md

@@ -86,20 +86,31 @@ invoke the tools they already depend upon.
 - Enable an ecosystem of users authoring plugins and sharing them
 - Reduce friction for publicly published whitebox `kustomization.yaml`s to generate Secrets
 
-
 ## Proposal
 
 Re-introduce `exec` Secret generators, but require the executables to have a whitelisted prefix
-and require a flag.
-
-- Introduce a flag `--secret-generator-exec-prefix` defaulted to `kustomize-sg-`
-- Introduce a flag `--enable-exec-secret-generator` defaulted to `false`
+and require a flag with the search location for the prefixes.
+
+- Introduce a flag `--sg-exec-prefix` defaulted to `kustomize-sg-`
+  - if set to the empty string, no prefix is used
+- Introduce a flag `--sg-exec-path` defaulted to ``
+  - Defines where to find the executables
+  - May be one of:
+    - (default) the empty string - disable exec secrets altogether
+    - `$PATH` - any executable in the user's path matching the prefix
+    - an absolute directory path - only executables matching the prefix in that directory
+    - a relative directory - only executables matching the prefix in that directory
 - Commands provided to generate Secrets *must* start with the prefix, or kustomize will exit 1
-- The enable flag *must* be provided if an exec Secret Generator is specified, or kustomize will exit 1
-- Command is executed from the `PATH` variable, it cannot be an absolute or relative path
-  - This is to prevent Carl from publishing a malicious generator with the kustomization.yaml and invoking it
-- Users can override the prefix with a string value with `len(value) > 2` - e.g. `sg-`
-  - Min length is to prevent users from providing an empty value, which would allow arbitrary command execution
+- The `sg-exec-path` flag *must* be provided if an exec Secret Generator is specified, or kustomize will exit 1
+- Command location is restricted to what is supplied in the `sg-exec-path` flag
+- Users can override the prefix with a string value of their choice.  May use an empty string.
+
+### Introduce new Command in Kustomize for printing out exec commands
+
+Add a new command in kustomize: `kustomize audit secret-generator`
+
+This will print out which commands (including args and flags) will be invoked when kustomize is run,
+without actually invoking them.  This will generally be helpful for users to view how secrets are generated.
 
 ## Risks and Mitigations
 
@@ -111,37 +122,27 @@ Required steps to exploit:
 
 - Alice executes (via `-k` or `kustomize`) an untrusted kustomization.yaml containing
   a SecretGenerator
-- Alice opted-in to generation by providing `--enable-exec-secret-generator`
-- Alice "installed" the targeted SecretGenerator locally on their PATH
-  - Added the command starting with `kustomize-sg-` (or whatever they provided via the flag) to her `PATH`
-- The installed SecretGenerator can be provided arguments in a way that can be exploited
-  - e.g. A command like `cat` would not be possible for Carl to exploit in a meaningful way
+- Alice opted-in to generation by providing `--sg-exec-path`
+- Either
+  - Option A
+    - Alice "installed" the targeted SecretGenerator locally on their PATH
+      - Added the command starting with `kustomize-sg-` (or whatever they provided via the flag) to her `PATH`
+    - Alice set the `sg-exec-path` to `$PATH`
+    - The installed SecretGenerator can be provided arguments in a way that can be exploited
+      - e.g. A command like `cat` would not be possible for Carl to exploit in a meaningful way
+  - Option B
+    - Alice set the `sg-exec-prefix` to empty
+    - Alice set the `sg-exec-path` to `$PATH`
 
 Analysis:
 
 This is a very low risk profile, with Alice having to install the targeted binary on their
 PATH as a kustomize plugin (using the name-prefix), the binary would have to be one that could
 be exploited by Carl, Alice would need to run kustomize against an untrusted `kustomization.yaml`,
-and Alice would need to provide the flag `--enable-exec-secret-generator`.  If Carl was able
+and Alice would need to provide the flag `--sg-exec-path` with `$PATH`.  If Carl was able
 to get Alice to perform these steps, he would likely be able to get her to run
 `$ curl <site> | bash` which would be a more effective technique.
 
-### Easy of Use
-
-**Risk:** This is too hard or confusing for users to use.
-
-Required steps to use:
-
-- Alice creates a script to generate the Secret called `kustomize-sg-my-cool-script.sh`
-- Alice adds the script to her PATH
-- Alice authors the `kustomization.yaml` with a SecretGenerator invoking the script
-- Alice runs kustomize with `--enable-exec-secret-generator`
-
-Analysis:
-
-There are a small number of simple and well defined steps that ensure Alice explicitly configures
-her environment so that kustomize can execute specific commands.
-
 ## Alternatives Considered
 
 Create a plugin mechanism similar to git or kubectl plugins and exec to plugins only.
@@ -175,16 +176,20 @@ Gather user feedback.  Determine if there are addressable gaps.
 - Verify that if the user specifies the prefix flag, the new prefix is used.
   - Commands with the new prefix should work
   - Commands with the default prefix should now fail
-- Verify that if the enable flag is not provided, kustomize will exit 1 if and exec generators are provided
+- Verify that if the dir flag is not provided, kustomize will exit 1 if and exec generators are provided
   - Even if they match the prefix
-- Verify absolute paths fail
-- Verify relative paths fail
+- Verify the dir $PATH value for the dir flag
+- Verify absolute and local values for the dir flag
 - Verify the happy case
 
 ### Documentation
 
 Update the "Kubectl Book" Secret Generator chapter.
 
+- Document security implications of no prefix
+- Document security implications of various paths (e.g. relative paths into an untrusted location)
+- Document that commands are executed when running `kustomize build`, `kubectl kustomize`, and `kubectl apply -k --dry-run`
+
 ## Implementation History
 
 (TODO add PR's here)