feat: add password flag for zarf tools gen-key “Relates to #4739”

[nicholas-rees]

Description

Adds password command line flags, and an interactive flag to allow easier scripting for zarf tools gen-key.

As suggested in #4739, I tried to mimic the behavior as close as possible to zarf registry login. However, by default, the registry login command runs interactively. So as to not change the current behavior and api, the tools gen-key command runs as interactive by default unless you pass --interactive=false. This behavior can be modified if it is desired to change the behavior. Just let me know.

Related Issue

Relates to #4739

Checklist before merging

• Test, docs, adr added or updated as needed
• Contributor Guide Steps followed

P.S. Let me know if you want me to fill out this checklist. I ran the pre-commit hooks, but obviously the ci in the actions is authoritative, so I don't want to check things before they're done. I also read and tried best to follow the Contributor guide. Let me know if anything is unsatisfactory, and I will do what I can to fix it.

[netlify]

✅ Deploy Preview for zarf-docs ready!

Name	Link
🔨 Latest commit	8064f41
🔍 Latest deploy log	https://app.netlify.com/projects/zarf-docs/deploys/69dff72b13cc6300088ed9dd
😎 Deploy Preview	https://deploy-preview-4747--zarf-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

❌ Patch coverage is 50.66667% with 37 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/cmd/zarf_tools.go	50.66%	32 Missing and 5 partials ⚠️

Files with missing lines	Coverage Δ
src/cmd/zarf_tools.go	37.72% <50.66%> (+4.99%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[brandtkeller]

Appreciative of this first pass - a few minor changes otherwise this is great. Just for local testing purposes I might recommend using a branch other than main for your development - but I could work-around it easily enough.

[nicholas-rees]

Appreciative of this first pass - a few minor changes otherwise this is great. Just for local testing purposes I might recommend using a branch other than main for your development - but I could work-around it easily enough.

To be honest, I read that in the contributing guide that you have short lived development branches with trunk based development, but forgot because of the way github requires you to fork to make a PR.

Since the PR is already made, I'm just going to sin boldly, and leave it be until this is finished--unless otherwise requested, for the purposes of reducing confusion at this point.

[nicholas-rees]

A few notes:

The biggest thing I that I want to make sure the force flag and interactive flag logic with the "keyfilesExist" check is appropriate.

I also rebased and force pushed, because I realized that when I started I didn't add my keyfile to my git commits. Sorry for that confusion. If you'd like me to squash to one commit, please let me know.

Also I looked into increasing the code coverage for this function, but I'd need a little more direction on how you would like that done. According to the tests page, it seems you would prefer e2e tests? But I want to be sure, before I go adding extra files to the repo.

I'm happy to do that work, but I would like just a tiny bit of direction.

[brandtkeller]

A few notes:

The biggest thing I that I want to make sure the force flag and interactive flag logic with the "keyfilesExist" check is appropriate.

I'll review this shortly.

I also rebased and force pushed, because I realized that when I started I didn't add my keyfile to my git commits. Sorry for that confusion. If you'd like me to squash to one commit, please let me know.

If it's not blocked by our branch protections then no worries.

Also I looked into increasing the code coverage for this function, but I'd need a little more direction on how you would like that done. According to the tests page, it seems you would prefer e2e tests? But I want to be sure, before I go adding extra files to the repo.

Testing is often dependent on some of the required infrastructure. Given this operation is isolated to the CLI and there is an existing zarf_tools_test.go file - that would be my first thought for coverage around testing this logic.

[nicholas-rees]

I wrote some tests for gen-key.

I anticipate the review.

If I've gone overboard on the testing let me know.

[brandtkeller]

This is taking shape! appreciate the iterations.

[nicholas-rees]

post got duplicated sorry.

[nicholas-rees]

One thing to note, when I disabled the the empty string for --password-stdin you can actually still:

❯ echo "" | build/zarf-mac-apple tools gen-key --password-stdin
2026-04-01 11:51:12 INF using config file location=/Users/nick/code/git_repos/github/zarf-dev/zarf/zarf-config.toml
2026-04-01 11:51:12 INF Successfully generated key pair private-key-path=cosign.key public-key-path=cosign.pub

but:

❯ printf "" | build/zarf-mac-apple tools gen-key --password-stdin
2026-04-01 11:52:02 INF using config file location=/Users/nick/code/git_repos/github/zarf-dev/zarf/zarf-config.toml
2026-04-01 11:52:02 ERR empty password disallowed when using --password-stdin

This is obviously because echo passes a \n , and I'm just checking for the empty string and not all whitespace characters. Obviously this is not a problem for the --password flag (see bottom).

I'm not sure how deep this should go for checking the errors, and I think maybe its not that great of a solution for example, if a shell variable isn't set or is set to an empty string:

❯ TESTVAR=''
❯ echo "${TESTVAR}" | build/zarf-mac-apple tools gen-key --password-stdin --force
2026-04-01 11:56:03 INF using config file location=/Users/nick/code/git_repos/github/zarf-dev/zarf/zarf-config.toml
2026-04-01 11:56:03 INF Successfully generated key pair private-key-path=cosign.key public-key-path=cosign.pub
❯ printf "${TESTVAR}" | build/zarf-mac-apple tools gen-key --password-stdin --force
2026-04-01 11:56:09 INF using config file location=/Users/nick/code/git_repos/github/zarf-dev/zarf/zarf-config.toml
2026-04-01 11:56:09 ERR empty password disallowed when using --password-stdin

if you set TESTVAR="$(<some-command>)", and that command fails, then you run into the same problem I mentioned above . . .

I could test for all whitespace, but I'm actually discovering that it seems like it might be legit to pass \t \n etc into a legit cosign password:

❯ printf "\t" | build/zarf-mac-apple tools gen-key --password-stdin --force
2026-04-01 12:02:36 INF using config file location=/Users/nick/code/git_repos/github/zarf-dev/zarf/zarf-config.toml
2026-04-01 12:02:36 INF Successfully generated key pair private-key-path=cosign.key public-key-path=cosign.pub
❯ cosign sign-blob --key cosign.key --bundle /dev/null test.txt > /dev/null
Enter password for private key:
Error: signing test.txt: getting keypair and token: getting signer: reading key: decrypt: encrypted: decryption failed
error during command execution: signing test.txt: getting keypair and token: getting signer: reading key: decrypt: encrypted: decryption failed

But I have no idea how you're supposed to enter the tab character in the prompt? XD. I have no idea how this should be handled? Is it a problem with cosign? Or something we should do about?

If cosign allows for it, it seems like its a problem with cosign? But it also doesn't feel right. Let me know your opinion.

Yeah, I think this is definitely a real issue even for --password . . .

❯ build/zarf-mac-apple tools gen-key --password="$(printf '\n\t\b')" --force
2026-04-01 12:16:38 INF using config file location=/Users/nick/code/git_repos/github/zarf-dev/zarf/zarf-config.toml
2026-04-01 12:16:38 INF Successfully generated key pair private-key-path=cosign.key public-key-path=cosign.pub
❯ cosign sign-blob --key cosign.key --bundle /dev/null test.txt > /dev/null
Enter password for private key:
Error: signing test.txt: getting keypair and token: getting signer: reading key: decrypt: encrypted: decryption failed
error during command execution: signing test.txt: getting keypair and token: getting signer: reading key: decrypt: encrypted: decryption failed

To be clear it's also a problem with the cosign package, they're doing even less checks than we are.

❯ echo "" | cosign generate-key-pair
Private key written to cosign.key
Public key written to cosign.pub
❯ cosign sign-blob --key cosign.key --bundle /dev/null test.txt > /dev/null
Enter password for private key:
Error: signing test.txt: getting keypair and token: getting signer: reading key: decrypt: encrypted: decryption failed
error during command execution: signing test.txt: getting keypair and token: getting signer: reading key: decrypt: encrypted: decryption failed
❯ printf "" | cosign generate-key-pair
Private key written to cosign.key
Public key written to cosign.pub
❯ cosign sign-blob --key cosign.key --bundle /dev/null test.txt > /dev/null
Enter password for private key:
Using payload from: test.txt
Wrote bundle to file /dev/null

[nicholas-rees]

I wrote a little password validation trying to whitelist only printable ascii characters. Definitely need a review on that to be sure its good.

It will also fail with echo every time, but not with printf.

I suppose I could just strip the newlines? I'm not sure what should be stripped and what shouldn't. I am neither going to confirm nor deny whether or not I use spaces in my passwords, but i definitely don't use newlines. . .

Someone might want to cat a file, so what should happen? Should new lines just be stripped, replaced with spaces? Personally, I think at this point, at least tell the user that its invalid, and they can use whatever utility or logic, as long as it doesn't contain non printable ascii. printf is way better than echo anyway. . .

[nicholas-rees]

I pulled in changes from main and repushed. Also, there was a slight issue with interactive mode that I fixed when I went back to make sure everything was working after the rebase.

[brandtkeller]

Appreciate the patience! Adding some additional thoughts.

[brandtkeller]

minor go-backs before approval. Testing locally with the suggested changes appears to be intuitive.

[brandtkeller]

You made a good callout previously for trimming newlines.

IE

echo "test" | ../build/zarf-mac-apple tools gen-key --password-stdin
2026-04-15 12:46:37 ERR invalid character (ascii code 0x0A) in password

I don't think this is entirely intuitive but I think we could keep this simple for now until other feedback is received.

Suggested change

	}
	}
	password = bytes.TrimSuffix(password, []byte("\n"))
	password = bytes.TrimSuffix(password, []byte("\r"))

[brandtkeller]

Might want to check formatting on this suggestion (or just replicate it yourself)

[nicholas-rees]

This concerns me. I'm happy to remove characters that shouldn't exist, but I think the correct approach is to simply take the validation script, and remove all bytes that shouldn't exist, basically printiable ascii+ space character (everything between 32-126, perhaps also rename it from validatePassword to sanitizePassword)
Like I said before, this is a cosign issue--not a zarf issue. Cosign allows you to assign any byte as a password. So I ask, why stop at new lines. Should a tab character be permitted?
My opinion, I'd leave it as it is. Its on the user to use cosign, and its how the actual cosign binary currently works. However, if you insist on modifying the behavior of cosign, I suggest giving me a list of all characters that should be allowed in a password, and I will remove them.
Your solution still leaves many characters being inputted into a password that will cause problems. I get that the validation script will catch them, but I'm unsure how its any less intutive. I think its less intuitive for echo to magically work than for it not to.

[brandtkeller]

Those are appropriate questions to ask and I think there could be a fair argument for it needing to be fixed in Cosign upstream.

I'd also wager this is some of the intention of the cosign PassFunc - but here we are validating eagerly before this is ever called.

So less from an extensive list of "what is correct" to asking "what is intuitive?".

I don't think echo -n is going to be the obvious choice but I also don't think we need an exhaustive list of what is permitted until we have signals to support their use. Replicating what docker does is where I was leaning towards for prior art.

Thoughts?

[nicholas-rees]

I have trouble with this because, what is intuitive to me is generally not intuitive to other people. I think modifying any of the data without informing the user can be a bit of an issue, which is why I made the validation function in the first place.

What if, we logged a warning which will strip a single \r and a single \n, informing the user that they shouldn't do that, but that we are going to modify their input to "make it work"? At least then we're being explicit about what is happening?

[brandtkeller]

we can certainly give it a try. One of the zarf personas is a low level IT user using zarf. If they were to run echo "mysecretpassword" | zarf tools gen-key --password-stdin would they understand why we're logging a warning for the existence of stripped characters that they never provided?

[nicholas-rees]

This is what is confusing me. If they didn't provide the \r \n, then how did the \r \n get there? If they just don't understand they're being put there, isn't this a great learning opportunity for low level it users?

[brandtkeller]

Also a good question - That's the POSIX/bash default behavior. echo is defined to print its arguments followed by a line terminator. Part of why we see trimming these characters by default across CLI's.

[nicholas-rees]

If its standard behavior then why would they think they didn't provide the \n? They did that when they chose to use echo.

Also, I'm not suggesting anyone ever use echo -n. That's not posix compliant either. I don't want to get into the weeds of posix compliance here, but the general recommendation is to printf because then you do have direct control as a user over the formatting of the output.

I leave the decision to you.

[brandtkeller]

My only concern is that the user experience aligns to what already exists. I mentioned the zarf tools registry login command originally as having behaviors we would want to replicate - and we see this pattern used here as well.

[brandtkeller]

@nicholas-rees friendly nudge - are you still interested (and have time) to see this completed? I think it's nearly there.

[brandtkeller]

Friendly nudge - this one is close but looking to determine whether this work is going to see completion or closing as stale to free up the issue.