From d7e1400487eacb9b41120a9b596450e688a41c11 Mon Sep 17 00:00:00 2001 From: Brian Majewski Date: Sun, 7 Jun 2026 08:44:02 -0700 Subject: [PATCH] feat: external entries via .secrets.json + legacy absorb + properties rail (EGB-677 stage 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - .secrets.json external[] drives push/pull: type 'properties' (alias of gradle-properties; blob suffix stays legacy-compatible in stage 1) and type 'file'; same charset rails as the legacy parser - push absorbs uncovered .secrets-files entries into the manifest (idempotent, gradle-properties → properties) with a delete hint - pull: manifest wins entirely; a coexisting .secrets-files warns as superseded instead of being silently ignored - basename rail generalized: properties targets must end '.properties' (was exact 'gradle.properties') — rc files/gitconfig still blocked; EGB-531 wrong-basename test updated for the sanctioned change --- secrets | 206 +++++++++++++++++++++++++++++++++++++++------ test/manifest.bats | 135 +++++++++++++++++++++++++++++ test/secrets.bats | 13 ++- 3 files changed, 322 insertions(+), 32 deletions(-) diff --git a/secrets b/secrets index e1d4f2c..e02e636 100755 --- a/secrets +++ b/secrets @@ -425,9 +425,17 @@ _parse_secrets_files_manifest() { _validate_external_target_path() { local p="$1" mtype="${2:-gradle-properties}" local base; base=$(basename "$p") - if [ "$mtype" = "gradle-properties" ] && [ "$base" != "gradle.properties" ]; then - echo "ERROR: $SECRETS_FILES_NAME: target basename must be 'gradle.properties' (got '$base'). Refusing." >&2 - return 1 + if [ "$mtype" = "gradle-properties" ]; then + # EGB-677: generalized from exact 'gradle.properties' to any + # '*.properties' basename — still blocks merging key=value lines + # into ~/.bashrc / ~/.gitconfig style targets. + case "$base" in + *.properties) ;; + *) + echo "ERROR: properties target basename must end in '.properties' (got '$base'). Refusing." >&2 + return 1 + ;; + esac fi case "$p" in *..*) echo "ERROR: $SECRETS_FILES_NAME: target path may not contain '..'. Refusing." >&2; return 1 ;; esac local home_real; home_real=$(cd -P "$HOME" 2>/dev/null && pwd -P) || home_real="$HOME" @@ -575,13 +583,11 @@ merge_gradle_keys() { # there is no usable manifest. Dies on unsafe targets or all-missing keys. push_external_files() { local root="$1" project="$2" pubkey="$3" - local manifest="$root/$SECRETS_FILES_NAME" - [ -e "$manifest" ] || return 1 - if [ -L "$manifest" ]; then - echo "WARNING: $manifest is a symlink; ignoring." >&2 - return 1 - fi - [ -f "$manifest" ] || return 1 + # Entries come from .secrets.json (EGB-677) plus any legacy + # .secrets-files entries the manifest doesn't cover yet. + local entries + entries=$(_external_entries_for_push "$root") + [ -n "$entries" ] || return 1 local pushed=0 mtype mpath mkeys while IFS=$'\t' read -r mtype mpath mkeys; do @@ -631,7 +637,7 @@ push_external_files() { rm -f "$tmp" info "Extracted $found key(s) from $mpath" pushed=$((pushed + 1)) - done < <(_parse_secrets_files_manifest "$manifest") + done <<< "$entries" [ "$pushed" -gt 0 ] } @@ -641,13 +647,11 @@ push_external_files() { # missing blobs rather than aborting the whole pull. pull_external_files() { local root="$1" project="$2" - local manifest="$root/$SECRETS_FILES_NAME" - [ -e "$manifest" ] || return 0 - if [ -L "$manifest" ]; then - echo "WARNING: $manifest is a symlink; ignoring." >&2 - return 0 - fi - [ -f "$manifest" ] || return 0 + # .secrets.json wins entirely when present (EGB-677); legacy + # .secrets-files only drives manifest-less projects. + local entries + entries=$(_external_entries_for_pull "$root") + [ -n "$entries" ] || return 0 local mtype mpath mkeys while IFS=$'\t' read -r mtype mpath mkeys; do @@ -709,7 +713,7 @@ pull_external_files() { echo "WARNING: failed to merge keys into $expanded — target left unchanged." >&2 fi rm -f "$tmp" - done < <(_parse_secrets_files_manifest "$manifest") + done <<< "$entries" } # ─── Manifest (.secrets.json) — EGB-677 store format v2, stage 1 ────── @@ -833,6 +837,133 @@ cmd_add() { info "Commit the manifest so other machines pick it up. To undo: edit $SECRETS_JSON_NAME and remove the entry." } +# Emit "\t\t" tuples from a .secrets.json external[] +# array — the same wire format _parse_secrets_files_manifest produces, so +# push_external_files / pull_external_files consume either source +# unchanged. JSON type 'properties' maps to the legacy tuple token +# 'gradle-properties' so blob suffixes (and existing store blobs) stay +# stable in stage 1. Applies the same conservative charset checks as the +# legacy parser — jq guarantees well-formed JSON, not safe VALUES. +_json_external_entries() { + local manifest="$1" + check_cmd jq + local etype epath ekeys + while IFS=$'\t' read -r etype epath ekeys; do + [ -n "$etype" ] || continue + case "$etype" in + properties|gradle-properties) + etype="gradle-properties" + if [ -z "$ekeys" ]; then + echo "WARNING: $SECRETS_JSON_NAME: properties entry '$epath' has no keys. Skipping." >&2 + continue + fi + ;; + file) + if [ -n "$ekeys" ]; then + echo "WARNING: $SECRETS_JSON_NAME: 'file' entries take no keys ('$epath' lists '$ekeys'). Skipping." >&2 + continue + fi + ;; + *) + echo "WARNING: $SECRETS_JSON_NAME: unknown external type '$etype' (supported: properties file). Skipping." >&2 + continue + ;; + esac + case "$epath" in + ''|*[!A-Za-z0-9/._~-]*|*..*) + echo "WARNING: $SECRETS_JSON_NAME: unsafe characters in external path '$epath'. Skipping." >&2 + continue + ;; + esac + case "$ekeys" in + *[!A-Za-z0-9._\ -]*) + echo "WARNING: $SECRETS_JSON_NAME: unsafe characters in key list for '$epath'. Skipping." >&2 + continue + ;; + esac + printf '%s\t%s\t%s\n' "$etype" "$epath" "$ekeys" + done < <(jq -r '.external // [] | .[] | [.type, .path, ((.keys // []) | join(" "))] | @tsv' "$manifest") +} + +# External tuples for PUSH: .secrets.json entries first, then legacy +# .secrets-files entries whose (type, path) the manifest doesn't cover — +# the absorb set, which cmd_push folds into the manifest after a +# successful push so the two sources converge. +_external_entries_for_push() { + local root="$1" + local json="$root/$SECRETS_JSON_NAME" legacy="$root/$SECRETS_FILES_NAME" + local seen="" t p k + if [ -f "$json" ] && [ ! -L "$json" ]; then + while IFS=$'\t' read -r t p k; do + [ -n "$t" ] || continue + printf '%s\t%s\t%s\n' "$t" "$p" "$k" + seen="$seen$t|$p"$'\n' + done < <(_json_external_entries "$json") + fi + if [ -e "$legacy" ]; then + if [ -L "$legacy" ]; then + echo "WARNING: $legacy is a symlink; ignoring." >&2 + elif [ -f "$legacy" ]; then + while IFS=$'\t' read -r t p k; do + [ -n "$t" ] || continue + case "$seen" in *"$t|$p"$'\n'*) continue ;; esac + printf '%s\t%s\t%s\n' "$t" "$p" "$k" + done < <(_parse_secrets_files_manifest "$legacy") + fi + fi +} + +# External tuples for PULL: the manifest wins entirely when present; +# legacy .secrets-files is only consulted in manifest-less projects. +_external_entries_for_pull() { + local root="$1" + local json="$root/$SECRETS_JSON_NAME" legacy="$root/$SECRETS_FILES_NAME" + if [ -f "$json" ] && [ ! -L "$json" ]; then + if [ -f "$legacy" ] && [ ! -L "$legacy" ]; then + echo "WARNING: $legacy is superseded by $SECRETS_JSON_NAME and was ignored on pull. Run 'secrets push' to absorb it, then delete it." >&2 + fi + _json_external_entries "$json" + return 0 + fi + [ -e "$legacy" ] || return 0 + if [ -L "$legacy" ]; then + echo "WARNING: $legacy is a symlink; ignoring." >&2 + return 0 + fi + [ -f "$legacy" ] && _parse_secrets_files_manifest "$legacy" + return 0 +} + +# JSON array of legacy .secrets-files entries NOT yet in the manifest — +# what cmd_push absorbs. gradle-properties becomes 'properties' on the +# JSON side. Parser warnings suppressed (push_external_files re-parses +# and warns once). +_legacy_absorb_json() { + local root="$1" + local json="$root/$SECRETS_JSON_NAME" legacy="$root/$SECRETS_FILES_NAME" + local out="[]" + if [ ! -f "$legacy" ] || [ -L "$legacy" ]; then + printf '%s' "$out" + return 0 + fi + local seen="" + if [ -f "$json" ] && [ ! -L "$json" ]; then + seen=$(jq -r '.external // [] | .[] | ((if .type == "properties" then "gradle-properties" else .type end) + "|" + .path)' "$json") + fi + local t p k s found jtype + while IFS=$'\t' read -r t p k; do + [ -n "$t" ] || continue + found=0 + while IFS= read -r s; do [ "$s" = "$t|$p" ] && { found=1; break; }; done <<< "$seen" + [ "$found" -eq 1 ] && continue + jtype="$t"; [ "$t" = "gradle-properties" ] && jtype="properties" + out=$(printf '%s' "$out" | jq --arg type "$jtype" --arg path "$p" --arg keys "$k" \ + '. + [if $type == "file" then {type: $type, path: $path} + else {type: $type, path: $path, keys: ($keys | split(" ") | map(select(length > 0)))} end]') + done < <(_parse_secrets_files_manifest "$legacy" 2>/dev/null) + printf '%s' "$out" +} + # Quietly emit "ws-dir/basename" for every env file in a package.json # workspace under . Emits nothing (and never dies) when is # not a workspace monorepo or jq is unavailable — plain `push` calls this @@ -1178,20 +1309,39 @@ cmd_push() { fi # ── Manifest write AFTER successful encryption (bootstrap ordering) ── - if [ "$count" -gt 0 ] && [ -n "$to_add" ] && [ "$frozen" = false ] \ - && { [ "$auto_add" = true ] || [ "$have_manifest" = false ]; }; then - local add_json - add_json=$(printf '%s' "$to_add" | jq -R -s 'split("\n") | map(select(length > 0))') + # Two independent reasons to write: dotenv auto-adds, and absorbing a + # legacy .secrets-files (gradle-properties → properties) so the two + # external sources converge on the manifest. + local absorbed_json="[]" n_absorbed=0 + if [ "$frozen" = false ]; then + absorbed_json=$(_legacy_absorb_json "$PWD") + n_absorbed=$(printf '%s' "$absorbed_json" | jq 'length') + fi + local write_adds=false + if [ -n "$to_add" ] && { [ "$auto_add" = true ] || [ "$have_manifest" = false ]; }; then + write_adds=true + fi + if [ "$did" -eq 1 ] && [ "$frozen" = false ] \ + && { [ "$write_adds" = true ] || [ "$n_absorbed" -gt 0 ]; }; then + local add_json="[]" + [ "$write_adds" = true ] && add_json=$(printf '%s' "$to_add" | jq -R -s 'split("\n") | map(select(length > 0))') if [ "$have_manifest" = true ]; then - jq --argjson add "$add_json" '.dotenv = ((.dotenv // []) + $add)' "$manifest" \ + jq --argjson add "$add_json" --argjson ext "$absorbed_json" \ + '.dotenv = ((.dotenv // []) + $add) | .external = ((.external // []) + $ext)' "$manifest" \ | _write_manifest_canonical "$manifest" || die "Failed to update $manifest" else - jq -n --argjson add "$add_json" '{version: '"$MANIFEST_VERSION"', dotenv: $add}' \ + jq -n --argjson add "$add_json" --argjson ext "$absorbed_json" \ + '{version: '"$MANIFEST_VERSION"', dotenv: $add} | if ($ext | length) > 0 then .external = $ext else . end' \ | _write_manifest_canonical "$manifest" || die "Failed to write $manifest" fi - while IFS= read -r e; do - [ -n "$e" ] && info "Added '$e' to $SECRETS_JSON_NAME" - done <<< "$to_add" + if [ "$write_adds" = true ]; then + while IFS= read -r e; do + [ -n "$e" ] && info "Added '$e' to $SECRETS_JSON_NAME" + done <<< "$to_add" + fi + if [ "$n_absorbed" -gt 0 ]; then + info "Absorbed $n_absorbed entr(y/ies) from $SECRETS_FILES_NAME into $SECRETS_JSON_NAME (gradle-properties → properties). $SECRETS_FILES_NAME can be deleted." + fi info "Commit the manifest so other machines pick it up. To undo an entry: edit $SECRETS_JSON_NAME (or use 'secrets push --frozen' to skip auto-add)." fi diff --git a/test/manifest.bats b/test/manifest.bats index 9800c72..d9e97ae 100644 --- a/test/manifest.bats +++ b/test/manifest.bats @@ -244,3 +244,138 @@ load test_helper [ "$status" -eq 1 ] [[ "$output" == *"project-relative"* ]] || false } + +# ─── C: legacy absorb + external entries via .secrets.json ───────────── + +# Local fixtures (mirror secrets.bats EGB-531/652 helpers) +m_gradle_src() { mkdir -p "$HOME/.gradle"; printf '%s' "$1" > "$HOME/.gradle/gradle.properties"; } +m_file_src() { mkdir -p "$HOME/keystores"; printf 'KS\x00\x01\x02\xffDATA\n' > "$HOME/keystores/upload.keystore"; } + +@test "push absorbs .secrets-files into .secrets.json (properties + file)" { + init_with_remote + m_gradle_src $'beaconClerkPkTest=pk_test_abc\n' + m_file_src + local dir="$WORK_DIR/absorbproj"; mkdir -p "$dir" + echo "K=v" > "$dir/.env" + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\nfile ~/keystores/upload.keystore\n' > "$dir/.secrets-files" + cd "$dir" + run "$SECRETS_BIN" push + [ "$status" -eq 0 ] + [[ "$output" == *"Absorbed"* ]] || false + run jq -r '.external | length' .secrets.json + [ "$output" = "2" ] + run jq -r '.external[] | select(.path == "~/.gradle/gradle.properties") | .type' .secrets.json + [ "$output" = "properties" ] + run jq -r '.external[] | select(.type == "file") | .path' .secrets.json + [ "$output" = "~/keystores/upload.keystore" ] + # stage 1: blob naming stays legacy-compatible + run bash -c "ls $SECRETS_DIR/absorbproj/external/*.gradle-properties.age" + [ "$status" -eq 0 ] +} + +@test "absorb is idempotent — second push adds no duplicate externals" { + init_with_remote + m_gradle_src $'beaconClerkPkTest=pk_test_abc\n' + local dir="$WORK_DIR/absorb2"; mkdir -p "$dir" + echo "K=v" > "$dir/.env" + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > "$dir/.secrets-files" + cd "$dir" + "$SECRETS_BIN" push >/dev/null 2>&1 + run "$SECRETS_BIN" push + [ "$status" -eq 0 ] + run jq -r '.external | length' .secrets.json + [ "$output" = "1" ] +} + +@test "external properties entry in .secrets.json drives push without .secrets-files" { + init_with_remote + m_gradle_src $'beaconClerkPkTest=pk_test_abc\n' + local dir="$WORK_DIR/jsonextproj"; mkdir -p "$dir" + printf '{"version":2,"external":[{"type":"properties","path":"~/.gradle/gradle.properties","keys":["beaconClerkPkTest"]}]}\n' > "$dir/.secrets.json" + cd "$dir" + run "$SECRETS_BIN" push jsonextproj + [ "$status" -eq 0 ] + [[ "$output" == *"Extracted 1 key"* ]] || false + run bash -c "ls $SECRETS_DIR/jsonextproj/external/*.gradle-properties.age" + [ "$status" -eq 0 ] +} + +@test "pull merges properties keys sourced from .secrets.json" { + init_with_remote + m_gradle_src $'beaconClerkPkTest=pk_test_abc\nunrelated=keep\n' + local dir="$WORK_DIR/jsonpull"; mkdir -p "$dir" + printf '{"version":2,"external":[{"type":"properties","path":"~/.gradle/gradle.properties","keys":["beaconClerkPkTest"]}]}\n' > "$dir/.secrets.json" + cd "$dir" + "$SECRETS_BIN" push jsonpull >/dev/null 2>&1 + m_gradle_src $'beaconClerkPkTest=STALE\nunrelated=keep\n' + run "$SECRETS_BIN" pull jsonpull + [ "$status" -eq 0 ] + run grep -c 'beaconClerkPkTest=pk_test_abc' "$HOME/.gradle/gradle.properties" + [ "$output" = "1" ] + run grep -c 'unrelated=keep' "$HOME/.gradle/gradle.properties" + [ "$output" = "1" ] +} + +@test "properties rail generalized: any *.properties basename is accepted" { + init_with_remote + mkdir -p "$HOME/.config" + printf 'apiKey=abc123\n' > "$HOME/.config/app.properties" + local dir="$WORK_DIR/genprops"; mkdir -p "$dir" + printf '{"version":2,"external":[{"type":"properties","path":"~/.config/app.properties","keys":["apiKey"]}]}\n' > "$dir/.secrets.json" + cd "$dir" + run "$SECRETS_BIN" push genprops + [ "$status" -eq 0 ] + [[ "$output" == *"Extracted 1 key"* ]] || false +} + +@test "properties rail still blocks a non-.properties target" { + init_with_remote + printf 'PATH=/evil\n' > "$HOME/.bashrc" + local dir="$WORK_DIR/evilprops"; mkdir -p "$dir" + printf '{"version":2,"external":[{"type":"properties","path":"~/.bashrc","keys":["PATH"]}]}\n' > "$dir/.secrets.json" + cd "$dir" + run "$SECRETS_BIN" push evilprops + [ "$status" -eq 1 ] + [[ "$output" == *".properties"* ]] || false +} + +@test "file entry via .secrets.json round-trips binary with mode 600" { + init_with_remote + m_file_src + local dir="$WORK_DIR/jsonfile"; mkdir -p "$dir" + printf '{"version":2,"external":[{"type":"file","path":"~/keystores/upload.keystore"}]}\n' > "$dir/.secrets.json" + cd "$dir" + "$SECRETS_BIN" push jsonfile >/dev/null 2>&1 + cp "$HOME/keystores/upload.keystore" "$TEST_TMPDIR/orig.keystore" + rm "$HOME/keystores/upload.keystore" + run "$SECRETS_BIN" pull jsonfile + [ "$status" -eq 0 ] + cmp -s "$HOME/keystores/upload.keystore" "$TEST_TMPDIR/orig.keystore" + local mode + mode=$(stat -f '%Lp' "$HOME/keystores/upload.keystore" 2>/dev/null || stat -c '%a' "$HOME/keystores/upload.keystore") + [ "$mode" = "600" ] +} + +@test "json file entry with keys is rejected with a warning" { + init_with_remote + m_file_src + local dir="$WORK_DIR/badfile"; mkdir -p "$dir" + echo "K=v" > "$dir/.env" + printf '{"version":2,"external":[{"type":"file","path":"~/keystores/upload.keystore","keys":["nope"]}]}\n' > "$dir/.secrets.json" + cd "$dir" + run "$SECRETS_BIN" push badfile + [ "$status" -eq 0 ] + [[ "$output" == *"no keys"* ]] || false + run bash -c "ls $SECRETS_DIR/badfile/external/*.file.age 2>/dev/null" + [ "$status" -ne 0 ] +} + +@test "pull warns that .secrets-files is superseded when .secrets.json exists" { + init_with_remote + create_project_dir superproj + "$SECRETS_BIN" push superproj >/dev/null 2>&1 + printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > .secrets-files + run "$SECRETS_BIN" pull superproj + [ "$status" -eq 0 ] + [[ "$output" == *"superseded"* ]] || false +} diff --git a/test/secrets.bats b/test/secrets.bats index 56f051e..5537a03 100644 --- a/test/secrets.bats +++ b/test/secrets.bats @@ -1217,15 +1217,17 @@ gradle_project() { [ "$mode" = "600" ] } -@test "EGB-531: target with wrong basename is refused" { +@test "EGB-531: target with non-.properties basename is refused" { + # EGB-677 generalized the rail from exact 'gradle.properties' to any + # '*.properties' basename — shell rc files and gitconfig stay blocked. init_with_remote mkdir -p "$HOME/.gradle" - printf 'beaconClerkPkTest=x\n' > "$HOME/.gradle/custom.properties" + printf 'beaconClerkPkTest=x\n' > "$HOME/.gradle/evil.sh" mkdir -p "$WORK_DIR/gproj"; cd "$WORK_DIR/gproj" - printf 'gradle-properties ~/.gradle/custom.properties beaconClerkPkTest\n' > .secrets-files + printf 'gradle-properties ~/.gradle/evil.sh beaconClerkPkTest\n' > .secrets-files run "$SECRETS_BIN" push gproj [ "$status" -eq 1 ] - [[ "$output" == *"gradle.properties"* ]] || false + [[ "$output" == *".properties"* ]] || false } @test "EGB-531: target outside HOME is refused" { @@ -1404,6 +1406,9 @@ gradle_project() { init_with_remote create_project_dir gproj "$SECRETS_BIN" push gproj >/dev/null 2>&1 + # EGB-677: drop the bootstrap .secrets.json so the legacy manifest path + # is exercised (with a manifest present, .secrets-files is superseded). + rm -f "$WORK_DIR/gproj/.secrets.json" printf 'gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest\n' > "$WORK_DIR/gproj/.secrets-files" cd "$WORK_DIR/gproj" run "$SECRETS_BIN" pull gproj