Skip to content

feat: rekey only specific identity #295

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.

Sign up for GitHub

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

Open
felixscheinost wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: rekey only specific identity #295

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions pkgs/agenix.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function show_help () {
echo '-h, --help show help'
# shellcheck disable=SC2016
echo '-e, --edit FILE edits FILE using $EDITOR'
echo '-r, --rekey re-encrypts all secrets with specified recipients'
echo '-r, --rekey [PUBLIC_KEY] re-encrypts all secrets with specified recipients'
Copy link
Contributor Author

@felixscheinost felixscheinost Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this makes sense here.

It makes parsing harder e.g. --rekey -v is currently broken.

Copy link

@benaryorg benaryorg Mar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you choose to take the other changes from this review:

Suggested change
echo '-r, --rekey [PUBLIC_KEY] re-encrypts all secrets with specified recipients'
echo '-r, --rekey [PUBLIC_KEY...] re-encrypts all secrets with specified recipients'

However that extends the length beyond the current space available so the entire thing needs fresh spacing.

echo '-d, --decrypt FILE decrypts FILE to STDOUT'
echo '-i, --identity identity to use when decrypting'
echo '-v, --verbose verbose output'
Expand Down Expand Up @@ -46,6 +46,7 @@ function err() {
test $# -eq 0 && (show_help && exit 1)

REKEY=0
REKEY_PUBLIC_KEY=
Copy link

@benaryorg benaryorg Mar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the shebang is already bash it makes perfect sense to use an array here to be able to provide multiple keys IMHO.

Suggested change
REKEY_PUBLIC_KEY=
REKEY_PUBLIC_KEY=()

nit (outside the scope of this PR tho): these are variables, not environment variables, they shouldn't be uppercase

DECRYPT_ONLY=0
DEFAULT_DECRYPT=(--decrypt)

Expand Down Expand Up @@ -77,6 +78,10 @@ while test $# -gt 0; do
;;
-r|--rekey)
shift
if test $# -gt 0; then
Copy link
Contributor Author

@felixscheinost felixscheinost Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Fix e.g. --rekey -v

Copy link

@benaryorg benaryorg Mar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could just keep the REKEY=1 below and not add anything here, instead handling the arguments in the * case.

Basically:

  -r|--rekey)
    shift
    REKEY=1
    ;;
# […]
  -*)
    show_help
    exit 1
  *)
    if test 1 = "$REKEY"; then
      REKEY_PUBLIC_KEY+=( "$1" )
      shift
    else
      show_help
      exit 1
    fi
    ;;

REKEY_PUBLIC_KEY="$1"
shift
fi
REKEY=1
;;
-d|--decrypt)
Expand Down Expand Up @@ -189,7 +194,22 @@ function edit {
}

function rekey {
FILES=$( (@nixInstantiate@ --json --eval -E "(let rules = import $RULES; in builtins.attrNames rules)" | @jqBin@ -r .[]) || exit 1)
if test ! -z "$REKEY_PUBLIC_KEY"; then
FILTER_EXPRESSION="builtins.elem \"$REKEY_PUBLIC_KEY\" rules.\${file}.publicKeys";
else
FILTER_EXPRESSION="true";
fi

RULES_EXPRESSION=$(cat <<EOF
Copy link
Contributor Author

@felixscheinost felixscheinost Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does using a here document make sense here? I tried fitting it all in the single line but that isn't pretty.

let
rules = import $RULES;
filter = file: $FILTER_EXPRESSION;
in
builtins.filter filter (builtins.attrNames rules)
EOF
)

FILES=$( (@nixInstantiate@ --json --eval -E "$RULES_EXPRESSION" | @jqBin@ -r .[]) || exit 1)
Comment on lines +197 to +212
Copy link

@benaryorg benaryorg Mar 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the other changes from my review this should use a loop over the keys.
Besides, using wordsplit for iteration is bad, this should use a loop (which is outside the code changes of this PR but not outside the scope IMHO). Otherwise having a space in any filename is a recipe for chaos.

Suggested change
if test ! -z "$REKEY_PUBLIC_KEY"; then
FILTER_EXPRESSION="builtins.elem \"$REKEY_PUBLIC_KEY\" rules.\${file}.publicKeys";
else
FILTER_EXPRESSION="true";
fi
RULES_EXPRESSION=$(cat <<EOF
let
rules = import $RULES;
filter = file: $FILTER_EXPRESSION;
in
builtins.filter filter (builtins.attrNames rules)
EOF
)
FILES=$( (@nixInstantiate@ --json --eval -E "$RULES_EXPRESSION" | @jqBin@ -r .[]) || exit 1)
if test "${#REKEY_PUBLIC_KEY[@]}" -gt 0; then
FILTER_EXPRESSION="let keys = [ $(printf \"%s\"\\n "${REKEY_PUBLIC_KEY[@]}" | paste -sd " ") ]; in builtins.any (key: builtins.elem key keys) rules.\${file}.publicKeys;
else
FILTER_EXPRESSION="true";
fi
RULES_EXPRESSION=$(cat <<EOF
let
rules = import $RULES;
filter = file: $FILTER_EXPRESSION;
in
builtins.filter filter (builtins.attrNames rules)
EOF
)
FILES=()
while read -r file; do
FILES+=( "$file" )
done < <(( @nixInstantiate@ --json --eval -E "$RULES_EXPRESSION" | @jqBin@ -r .[]) || exit 1)

Not sure if the extra parens around the instantiate are needed give that pipefail is set.

nit (outside the scope of this PR): someone should clean up the indents in this file, choose: either two or four spaces, or ideally if you want people to be able to use their preferred width (and also aid accessibility for screen readers and such) use hard tabs instead, but choose one of those


for FILE in $FILES
do
Expand Down