diff --git a/CHANGELOG.md b/CHANGELOG.md index b96af01..214b95c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to a four-digit MAJOR.MINOR.PATCH.MICRO version scheme. - **Sync designated keys from external files (Gradle properties).** A new committed `.secrets-files` manifest lets `secrets` track specific keys from files that live *outside* the project root — the motivating case being `~/.gradle/gradle.properties`, where Android builds read Clerk publishable keys (`beaconClerkPkTest`, `beaconClerkPkLive`) that Android Studio's GUI builds can only get from that persistent global file, not from terminal env vars. One entry per line: `gradle-properties ~/.gradle/gradle.properties beaconClerkPkTest beaconClerkPkLive`. (EGB-531) - **push** extracts only the named keys and encrypts them under `/external/` in the store. - - **pull** *merges* those keys into the target file, preserving every unrelated key, comment, and line order. An existing managed key is updated in place; the target is backed up to `gradle.properties.secrets-bak` before the first merge. + - **pull** *merges* those keys into the target file, preserving every unrelated key, comment, and line order. An existing managed key is updated in place; the target is backed up to `gradle.properties.secrets-bak` before each merge. - `secrets which` reads back the parsed manifest; `secrets list` shows `[external]` entries; `secrets rekey` re-encrypts external blobs alongside dotenv ones. - Backward compatible: no `.secrets-files` → identical behavior to before. @@ -28,7 +28,7 @@ and this project adheres to a four-digit MAJOR.MINOR.PATCH.MICRO version scheme. ### Tests -- 80 → 112 (+32). New coverage: manifest parse/read-back, key extraction across `=`/`:`/space separators, merge (preserve unrelated/comments/order, substring-key isolation, sed-metachar value round-trip, duplicate-key collapse, continuation-line safety, idempotency), path validation (wrong basename, outside `$HOME`, symlinked target, symlinked parent dir), first-create mode `600`, manifest injection/symlink/unsafe-key rejection, rekey round-trip of external blobs, glob isolation (blob not leaked to cwd), `list` surfacing, pre-commit blocking plaintext `gradle.properties`, workspace (`push -w`/`pull -w`) external sync, multi-entry manifests, partial-key push warnings, missing-blob pull warnings, source-side comment/continuation skipping, and backward compatibility. +- 80 → 113 (+33). New coverage: manifest parse/read-back, key extraction across `=`/`:`/space separators, merge (preserve unrelated/comments/order, substring-key isolation, sed-metachar value round-trip, duplicate-key collapse, continuation-line safety, idempotency), path validation (wrong basename, outside `$HOME`, symlinked target, symlinked parent dir), first-create mode `600`, manifest injection/symlink/unsafe-key rejection, rekey round-trip of external blobs, glob isolation (blob not leaked to cwd), `list` surfacing, pre-commit blocking plaintext `gradle.properties`, workspace (`push -w`/`pull -w`) external sync, multi-entry manifests, partial-key push warnings, missing-blob pull warnings, source-side comment/continuation skipping, and backward compatibility. ## [0.1.1.0] - 2026-05-09 diff --git a/CLAUDE.md b/CLAUDE.md index c3e001c..e31f76a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,7 +37,7 @@ Single bash script (`secrets`) with subcommands: init, push, pull, list, rm, rek secrets # CLI script (~600 lines bash) hooks/pre-commit # Pre-commit hook template test/ - secrets.bats # bats-core test suite (112 tests) + secrets.bats # bats-core test suite (113 tests) test_helper.bash # Shared setup/teardown README.md # User-facing documentation CLAUDE.md # This file @@ -67,8 +67,8 @@ The active store directory is picked by `resolve_store()` using these rules, hig Key design decisions (all driven by /autoplan review): - **Wire-in is at command scope** (`cmd_push`/`cmd_pull`), via `push_external_files` / `pull_external_files`, **not** inside `push_dir_to_project` / `pull_project_to_dir` (those loop per-workspace and `pull_project_to_dir` uses stdout as a data channel). -- **Storage:** blobs live in `$SECRETS_DIR//external/.gradle-properties.age`. The `external/` subdir keeps them out of the existing non-recursive `*.age` / `.*.age` globs (pull, list, rekey), so the old dotenv path can never decrypt a blob into cwd. `cmd_rekey` and `cmd_list` recurse into `external/` explicitly (rekey MUST, or the blob is orphaned after rotation = data loss). `` = manifest path token with non-`[A-Za-z0-9._-]` chars → `_`, so it's machine-independent. -- **Merge is pure bash, no `sed`/regex** (`merge_gradle_keys`): exact-string key comparison (avoids `beaconClerkPk` vs `beaconClerkPkTest` substring bug), value treated as opaque literal (survives `& \ /` in values). Updates a managed key in place at its first occurrence, collapses duplicates, appends new keys, preserves unrelated lines/comments/order. Continuation lines (trailing odd backslashes, tracked by `_trailing_bs_odd`) are never matched as keys. Atomic write: temp in the same dir → `chmod` to match (or `600` on create) → `mv`. Backs up to `.secrets-bak` before the first merge. +- **Storage:** blobs live in `$SECRETS_DIR//external/.gradle-properties.age`. The `external/` subdir keeps them out of the existing non-recursive `*.age` / `.*.age` globs (pull, list, rekey), so the old dotenv path can never decrypt a blob into cwd. `cmd_rekey` and `cmd_list` recurse into `external/` explicitly (rekey MUST, or the blob is orphaned after rotation = data loss). `` = manifest path token with non-`[A-Za-z0-9._-]` chars → `_`, plus a `cksum` suffix of the original path so paths that clean to the same string (`a/b` vs `a_b`) don't collide. Machine-independent (derived from the committed manifest token, not the expanded path). +- **Merge is pure bash, no `sed`/regex** (`merge_gradle_keys`): exact-string key comparison (avoids `beaconClerkPk` vs `beaconClerkPkTest` substring bug), value treated as opaque literal (survives `& \ /` in values). Updates a managed key in place at its first occurrence, collapses duplicates, appends new keys, preserves unrelated lines/comments/order. Continuation lines (trailing odd backslashes, tracked by `_trailing_bs_odd`) are never matched as keys. Atomic write: temp in the same dir → `chmod` to match (or `600` on create) → `mv`. Backs up to `.secrets-bak` before each merge. - **Properties separator parsing** (`_props_get`): key ends at the first `=`, `:`, or whitespace (after lstrip); handles `key=value`, `key = value`, `key:value`, `key value`; last definition wins. - **Security:** the write target comes from a committed file, so `_validate_external_target_path` locks it down — basename must be `gradle.properties`, must resolve inside `$HOME` (deepest-existing-ancestor resolved, symlink target/parent refused, `..` rejected). This blocks a malicious manifest from appending decrypted keys to `~/.gitconfig`/`~/.bashrc`. `_parse_secrets_files_manifest` rejects shell metacharacters/control chars in path and keys (path allows `[A-Za-z0-9/._~-]` only; keys allow `[A-Za-z0-9._-]` + space), mirrors the `.secrets-store` posture (no shell expansion, symlinked manifest skipped). - **Plaintext tradeoff (accepted, documented):** merged keys are permanent plaintext in the target; `secrets clear` does not remove them. Fine for the Clerk *publishable* keys this was built for; not for high-value secrets (use `secrets run` + `.env`). diff --git a/README.md b/README.md index ded1889..2e23b41 100644 --- a/README.md +++ b/README.md @@ -385,7 +385,7 @@ secrets pull # ==> Merged 2 key(s) into ~/.gradle/gradle.properties (beaconClerkPkTest, beaconClerkPkLive) ``` -`secrets pull` merges those keys into the local `~/.gradle/gradle.properties`, leaving every other key untouched. If a managed key already exists, its value is updated in place; comments, ordering, and unrelated entries are preserved. The file is backed up to `gradle.properties.secrets-bak` before the first merge. +`secrets pull` merges those keys into the local `~/.gradle/gradle.properties`, leaving every other key untouched. If a managed key already exists, its value is updated in place; comments, ordering, and unrelated entries are preserved. The file is backed up to `gradle.properties.secrets-bak` before each merge. > **Note:** unlike `.env` files, merged Gradle keys are written as **permanent plaintext** into the target file — `secrets clear` does **not** remove them. This is appropriate for publishable / low-secrecy values (like Clerk publishable keys, `pk_*`). For high-value secrets that should never sit on disk, use `secrets run` with a `.env` instead. @@ -432,7 +432,7 @@ For complete rotation with no historical exposure, create a fresh `~/.secrets/` ## Development ```bash -# Run the test suite (112 tests) +# Run the test suite (113 tests) brew install bats-core bats test/secrets.bats ``` diff --git a/secrets b/secrets index d47ba01..76eb7c4 100755 --- a/secrets +++ b/secrets @@ -464,9 +464,14 @@ _props_get() { } # Turn a manifest path token into a machine-independent blob slug. +# A bare char-replace would collide (e.g. a/b and a_b both → a_b), so append a +# checksum of the original path to keep distinct targets' blobs distinct. _secrets_files_slug() { local p="$1" - printf '%s' "${p//[!A-Za-z0-9._-]/_}" + local clean="${p//[!A-Za-z0-9._-]/_}" + local sum + sum=$(printf '%s' "$p" | cksum | cut -d' ' -f1) + printf '%s-%s' "$clean" "$sum" } # Merge managed key=value lines (from $2) into target file $1, preserving @@ -569,8 +574,16 @@ push_external_files() { local found=0 k v for k in $mkeys; do if v=$(_props_get "$expanded" "$k"); then - printf '%s=%s\n' "$k" "$v" >> "$tmp" - found=$((found + 1)) + if _trailing_bs_odd "$v"; then + # A trailing odd backslash means a multi-line (continuation) value. + # We only support single-line values; syncing this would write a + # dangling backslash that turns the next target line into a + # continuation and corrupts the file. Skip it loudly. + echo "WARNING: key '$k' in $mpath has a multi-line (continuation) value — not supported, skipping." >&2 + else + printf '%s=%s\n' "$k" "$v" >> "$tmp" + found=$((found + 1)) + fi else echo "WARNING: key '$k' not found in $mpath — not synced. Set it locally first, or remove it from $SECRETS_FILES_NAME." >&2 fi @@ -622,17 +635,21 @@ pull_external_files() { rm -f "$tmp" die "Decryption failed for external target $mpath." fi - local k + local k k_esc for k in $mkeys; do - grep -q "^$k=" "$tmp" 2>/dev/null || echo "WARNING: '$k' listed in $SECRETS_FILES_NAME but absent from synced data for $mpath." >&2 + # Escape regex-special '.' so e.g. systemProp.foo can't match systemPropXfoo. + k_esc=$(printf '%s' "$k" | sed 's/\./\\./g') + grep -q "^$k_esc=" "$tmp" 2>/dev/null || echo "WARNING: '$k' listed in $SECRETS_FILES_NAME but absent from synced data for $mpath." >&2 done if [ ! -d "$(dirname "$expanded")" ]; then echo "WARNING: creating $(dirname "$expanded")" >&2 fi - local count; count=$(grep -c '=' "$tmp" 2>/dev/null || echo 0) + local count; count=$(grep -c '=' "$tmp" 2>/dev/null || true); count=${count:-0} if merge_gradle_keys "$expanded" "$tmp"; then local klist; klist=$(printf '%s' "$mkeys" | tr ' ' ',' | sed 's/,/, /g') info "Merged $count key(s) into $expanded ($klist)" + else + echo "WARNING: failed to merge keys into $expanded — target left unchanged." >&2 fi rm -f "$tmp" done < <(_parse_secrets_files_manifest "$manifest") diff --git a/test/secrets.bats b/test/secrets.bats index bd95488..2119c40 100644 --- a/test/secrets.bats +++ b/test/secrets.bats @@ -1451,6 +1451,18 @@ gradle_project() { [[ "$output" == *"symlink"* ]] } +@test "EGB-531: push skips a multi-line (continuation) managed value with a warning" { + init_with_remote + mkdir -p "$HOME/.gradle" + # beaconClerkPkTest has a continuation value (trailing backslash); Live is single-line + printf '%s' $'beaconClerkPkTest=part1\\\npart2\nbeaconClerkPkLive=fine\n' > "$HOME/.gradle/gradle.properties" + gradle_project gproj + run "$SECRETS_BIN" push gproj + [ "$status" -eq 0 ] + [[ "$output" == *"multi-line"* ]] + [[ "$output" == *"Extracted 1 key"* ]] +} + @test "EGB-531: push skips comment and continuation lines in source" { init_with_remote mkdir -p "$HOME/.gradle"