Delete the keychain item or Secure Enclave key of a failed unlocker add (closes #89)

A Secure Enclave unlocker add gets the long-term key before it creates
the Secure Enclave key, so a wrong passphrase creates none, and deletes
the key if encrypting with it or writing the unlocker then fails. A
keychain unlocker add writes all of the unlocker's files before it
stores the keychain item, and deletes the item if moving the unlocker
into place then fails. A failure to delete is reported along with the
original error.

These files build only on macOS: the code is type-checked and linted
from Linux by script/lint-darwin; the new tests run only on a Mac.

Model: opus-5-5

AGENTS.md

@@ -4,160 +4,154 @@ Version: 2025-06-08
 
 # Instructions and Contextual Information
 
-- Be direct, robotic, expert, accurate, and professional.
+* Be direct, robotic, expert, accurate, and professional.
 
-- Do not butter me up or kiss my ass.
+* Do not butter me up or kiss my ass.
 
-- Come in hot with strong opinions, even if they are contrary to the direction I
+* Come in hot with strong opinions, even if they are contrary to the
-  am headed.
+  direction I am headed.
 
-- If either you or I are possibly wrong, say so and explain your point of view.
+* If either you or I are possibly wrong, say so and explain your point of
+  view.
 
-- Point out great alternatives I haven't thought of, even when I'm not asking
+* Point out great alternatives I haven't thought of, even when I'm not
-  for them.
+  asking for them.
 
-- Treat me like the world's leading expert in every situation and every
+* Treat me like the world's leading expert in every situation and every
   conversation, and deliver the absolute best recommendations.
 
-- I want excellence, so always be on the lookout for divergences from good data
+* I want excellence, so always be on the lookout for divergences from good
-  model design or best practices for object oriented development.
+  data model design or best practices for object oriented development.
 
-- IMPORTANT: This is production code, not a research or teaching exercise.
+* IMPORTANT: This is production code, not a research or teaching exercise.
   Deliver professional-level results, not prototypes.
 
-- Please read and understand the `README.md` file in the root of the repo for
+* Please read and understand the `README.md` file in the root of the repo
-  project-specific contextual information, including development policies,
+  for project-specific contextual information, including development
-  practices, and current implementation status.
+  policies, practices, and current implementation status.
 
-- Be proactive in suggesting improvements or refactorings in places where we
+* Be proactive in suggesting improvements or refactorings in places where we
   diverge from best practices for clean, modular, maintainable code.
 
 # Policies
 
 1. Before committing, tests must pass (`make test`), linting must pass
-   (`make lint`), and code must be formatted (`make fmt`). For go, those
+   (`make lint`), and code must be formatted (`make fmt`).  For go, those
    makefile targets should use `go fmt` and `go test -v ./...` and
-   `golangci-lint run`. When you think your changes are complete, rather than
+   `golangci-lint run`.  When you think your changes are complete, rather
-   making three different tool calls to check, you can just run
+   than making three different tool calls to check, you can just run `make
-   `make test && make fmt && make lint` as a single tool call which will save
+   test && make fmt && make lint` as a single tool call which will save
    time.
 
-2. Always write a `Makefile` with the default target being `test`, and with a
+2. Always write a `Makefile` with the default target being `test`, and with
-   `fmt` target that formats the code. The `test` target should run all tests in
+   a `fmt` target that formats the code.  The `test` target should run all
-   the project, and the `fmt` target should format the code. `test` should also
+   tests in the project, and the `fmt` target should format the code.
-   have a prerequisite target `lint` that should run any linters that are
+   `test` should also have a prerequisite target `lint` that should run any
-   configured for the project.
+   linters that are configured for the project.
 
-3. After each completed bugfix or feature, the code must be committed. Do all of
+3. After each completed bugfix or feature, the code must be committed.  Do
-   the pre-commit checks (test, lint, fmt) before committing, of course.
+   all of the pre-commit checks (test, lint, fmt) before committing, of
+   course.
 
 4. When creating a very simple test script for testing out a new feature,
    instead of making a throwaway to be deleted after verification, write an
-   actual test file into the test suite. It doesn't need to be very big or
+   actual test file into the test suite.  It doesn't need to be very big or
    complex, but it should be a real test that can be run.
 
 5. When you are instructed to make the tests pass, DO NOT delete tests, skip
-   tests, or change the tests specifically to make them pass (unless there is a
+   tests, or change the tests specifically to make them pass (unless there
-   bug in the test). This is cheating, and it is bad. You should only be
+   is a bug in the test).  This is cheating, and it is bad.  You should only
-   modifying the test if it is incorrect or if the test is no longer relevant.
+   be modifying the test if it is incorrect or if the test is no longer
-   In almost all cases, you should be fixing the code that is being tested, or
+   relevant.  In almost all cases, you should be fixing the code that is
-   updating the tests to match a refactored implementation.
+   being tested, or updating the tests to match a refactored implementation.
 
 6. When dealing with dates and times or timestamps, always use, display, and
-   store UTC. Set the local timezone to UTC on startup. If the user needs to see
+   store UTC.  Set the local timezone to UTC on startup.  If the user needs
-   the time in a different timezone, store the user's timezone in a separate
+   to see the time in a different timezone, store the user's timezone in a
-   field and convert the UTC time to the user's timezone when displaying it. For
+   separate field and convert the UTC time to the user's timezone when
-   internal use and internal applications and administrative purposes, always
+   displaying it.  For internal use and internal applications and
-   display UTC.
+   administrative purposes, always display UTC.
 
 7. Always write tests, even if they are extremely simple and just check for
-   correct syntax (ability to compile/import). If you are writing a new feature,
+   correct syntax (ability to compile/import). If you are writing a new
-   write a test for it. You don't need to target complete coverage, but you
+   feature, write a test for it.  You don't need to target complete
-   should at least test any new functionality you add. If you are fixing a bug,
+   coverage, but you should at least test any new functionality you add.  If
-   write a test first that reproduces the bug, and then fix the bug in the code.
+   you are fixing a bug, write a test first that reproduces the bug, and
+   then fix the bug in the code.
 
-8. When implementing new features, be aware of potential side-effects (such as
+8. When implementing new features, be aware of potential side-effects (such
-   state files on disk, data in the database, etc.) and ensure that it is
+   as state files on disk, data in the database, etc.) and ensure that it is
    possible to mock or stub these side-effects in tests.
 
-9. Always use structured logging. Log any relevant state/context with the
+9. Always use structured logging.  Log any relevant state/context with the
-   messages (but do not log secrets). If stdout is not a terminal, output the
+   messages (but do not log secrets).  If stdout is not a terminal, output
-   structured logs in jsonl format.
+   the structured logs in jsonl format.
 
 10. Avoid using bare strings or numbers in code, especially if they appear
-    anywhere more than once. Always define a constant (usually at the top of the
+    anywhere more than once.  Always define a constant (usually at the top
-    file) and give it a descriptive name, then use that constant in the code
+    of the file) and give it a descriptive name, then use that constant in
-    instead of the bare string or number.
+    the code instead of the bare string or number.
 
 11. You do not need to summarize your changes in the chat after making them.
-    Making the changes and committing them is sufficient. If anything out of the
+    Making the changes and committing them is sufficient.  If anything out
-    ordinary happened, please explain it, but in the normal case where you found
+    of the ordinary happened, please explain it, but in the normal case
-    and fixed the bug, or implemented the feature, there is no need for the
+    where you found and fixed the bug, or implemented the feature, there is
-    end-of-change summary.
+    no need for the end-of-change summary.
 
-12. Do not create additional files in the root directory of the project without
+12. Do not create additional files in the root directory of the project
-    asking permission first. Configuration files, documentation, and build files
+    without asking permission first. Configuration files, documentation, and
-    are acceptable in the root, but source code and other files should be
+    build files are acceptable in the root, but source code and other files
-    organized in appropriate subdirectories.
+    should be organized in appropriate subdirectories.
 
 ## Python-Specific Guidelines
 
-1. **Type Annotations (UP006)**: Use built-in collection types directly for type
+1. **Type Annotations (UP006)**: Use built-in collection types directly for type annotations instead of importing from `typing`. This avoids the UP006 linter error.
-   annotations instead of importing from `typing`. This avoids the UP006 linter
+   
-   error.
+   **Good (modern Python 3.9+):**
-
+   ```python
-    **Good (modern Python 3.9+):**
+   def process_items(items: list[str]) -> dict[str, int]:
-
+       counts: dict[str, int] = {}
-    ```python
+       return counts
-    def process_items(items: list[str]) -> dict[str, int]:
+   ```
-        counts: dict[str, int] = {}
+   
-        return counts
+   **Avoid (triggers UP006):**
-    ```
+   ```python
-
+   from typing import List, Dict
-    **Avoid (triggers UP006):**
+   
-
+   def process_items(items: List[str]) -> Dict[str, int]:
-    ```python
+       counts: Dict[str, int] = {}
-    from typing import List, Dict
+       return counts
-
+   ```
-    def process_items(items: List[str]) -> Dict[str, int]:
+   
-        counts: Dict[str, int] = {}
+   For optional types, use the `|` operator instead of `Union`:
-        return counts
+   ```python
-    ```
+   # Good
-
+   def get_value(key: str) -> str | None:
-    For optional types, use the `|` operator instead of `Union`:
+       return None
-
+   
-    ```python
+   # Avoid
-    # Good
+   from typing import Optional, Union
-    def get_value(key: str) -> str | None:
+   def get_value(key: str) -> Optional[str]:
-        return None
+       return None
-
+   ```
-    # Avoid
-    from typing import Optional, Union
-    def get_value(key: str) -> Optional[str]:
-        return None
-    ```
 
 2. **Import Organization**: Follow the standard Python import order:
-    - Standard library imports
+   - Standard library imports
-    - Third-party imports
+   - Third-party imports
-    - Local application imports
+   - Local application imports
-
+   
-    Each group should be separated by a blank line.
+   Each group should be separated by a blank line.
 
 ## Go-Specific Guidelines
 
-1. **No `panic`, `log.Fatal`, or `os.Exit` in library code.** Always propagate
+1. **No `panic`, `log.Fatal`, or `os.Exit` in library code.** Always propagate errors via return values.
-   errors via return values.
 
-2. **Constructors return `(*T, error)`, not just `*T`.** Callers must handle
+2. **Constructors return `(*T, error)`, not just `*T`.** Callers must handle errors, not crash.
-   errors, not crash.
 
 3. **Wrap errors** with `fmt.Errorf("context: %w", err)` for debuggability.
 
-4. **Never modify linter config** (`.golangci.yml`) to suppress findings. Fix
+4. **Never modify linter config** (`.golangci.yml`) to suppress findings. Fix the code.
-   the code.
 
-5. **All PRs must pass `make check` with zero failures.** No exceptions, no
+5. **All PRs must pass `make check` with zero failures.** No exceptions, no "pre-existing issue" excuses.
-   "pre-existing issue" excuses.
 
 6. **Pin external dependencies by commit hash**, not mutable tags.

README.md

@@ -1,69 +1,72 @@
 # secret - Local Secret Manager
 
-## Description
+secret is a command-line local secret manager that implements a hierarchical
+key architecture for storing and managing sensitive data. It supports
+multiple vaults, various unlock mechanisms, and provides secure storage
+using the `age` encryption library.
 
-`secret` is a WTFPL-licensed Go command-line local secret manager by
+It could be used as password manager, but was not designed as such.  I
-[@sneak](https://sneak.berlin) that implements a hierarchical key architecture
+created it to scratch an itch for a secure key/value store for replacing a
-for storing and managing sensitive data. It supports multiple vaults, various
+bunch of pgp-encrypted files in a directory structure.
-unlock mechanisms, and provides secure storage using the `age` encryption
-library.
 
-## Getting Started
+## Core Architecture
-
-Build from source, then install the binary as `~/bin/secret`:
-
-```bash
-git clone https://git.eeqj.de/sneak/secret.git
-cd secret
-make build     # writes the binary to ./secret
-make install   # builds it and copies it to ~/bin/secret
-```
-
-Generate a mnemonic, create the default vault, then store and read a secret:
-
-```bash
-secret generate mnemonic   # prints a new BIP39 mnemonic; write it down
-secret init                # asks for that mnemonic and an unlocker passphrase
-echo "my-password" | secret add myservice/password
-secret get myservice/password
-```
-
-## Rationale
-
-I created `secret` to scratch an itch: I wanted a secure key/value store to
-replace a bunch of PGP-encrypted files in a directory structure. It could be
-used as a password manager, but was not designed as one.
-
-## Design
 
 ### Three-Layer Key Hierarchy
 
 Secret implements a three-layer key architecture:
 
-1. **Long-term Keys**: Derived from BIP39 mnemonic phrases, these provide the
+1. **Long-term Keys**: Derived from BIP39 mnemonic phrases, these provide
-   foundation for all encryption
+   the foundation for all encryption
-2. **Unlockers**: Short-term keys that encrypt the long-term keys, supporting
+2. **Unlockers**: Short-term keys that encrypt the long-term keys,
-   multiple authentication methods
+   supporting multiple authentication methods
-3. **Version-specific Keys**: Per-version keys that encrypt individual secret
+3. **Version-specific Keys**: Per-version keys that encrypt individual
-   values
+   secret values
 
 ### Version Management
 
 Each secret maintains a history of versions, with each version having:
-
 - Its own encryption key pair
-- Metadata including creation time and validity period, encrypted to the
+- Metadata (unencrypted) including creation time and validity period
-  version's key pair
 - Immutable value storage
-
+- Atomic version switching via symlink updates
-The secret's `current` file names its current version. Switching versions
-replaces that file in one rename, so it is never half-written.
 
 ### Vault System
 
-Vaults provide logical separation of secrets, each with its own long-term key
+Vaults provide logical separation of secrets, each with its own long-term
-and unlocker set. This allows for complete isolation between different contexts
+key and unlocker set. This allows for complete isolation between different
-(work, personal, projects).
+contexts (work, personal, projects).
+
+## Installation
+
+Build from source:
+```bash
+git clone <repository>
+cd secret
+make build
+```
+
+## Quick Start
+
+1. **Initialize the secret manager**:
+   ```bash
+   secret init
+   ```
+   This creates the default vault and prompts for a BIP39 mnemonic phrase.
+
+2. **Generate a mnemonic** (if needed):
+   ```bash
+   secret generate mnemonic
+   ```
+
+3. **Add a secret**:
+   ```bash
+   echo "my-password" | secret add myservice/password
+   ```
+
+4. **Retrieve a secret**:
+   ```bash
+   secret get myservice/password
+   ```
 
 ## Commands Reference
 
@@ -71,10 +74,10 @@ and unlocker set. This allows for complete isolation between different contexts
 
 `secret rm`, `secret version rm`, `secret vault remove` and
 `secret unlocker remove` destroy data that exists nowhere else. On a terminal
-each one first asks `[y/N]`, naming exactly what it is about to remove, and goes
+each one first asks `[y/N]`, naming exactly what it is about to remove, and
-ahead only on `y` or `yes`; any other answer, a bare Enter included, cancels and
+goes ahead only on `y` or `yes`; any other answer, a bare Enter included,
-removes nothing. The question is asked only after the command's checks have
+cancels and removes nothing. The question is asked only after the command's
-passed, and before it changes anything.
+checks have passed, and before it changes anything.
 
 Whether to ask is decided by stdin, where the answer is read from, so
 `secret rm foo | tee log` still asks. When stdin is not a terminal, as in a
@@ -93,7 +96,6 @@ Initializes the secret manager with a default vault. Prompts for a BIP39
 mnemonic phrase and creates the initial directory structure.
 
 **Environment Variables:**
-
 - `SB_SECRET_MNEMONIC`: Pre-set mnemonic phrase
 - `SB_UNLOCK_PASSPHRASE`: Pre-set unlock passphrase
 
@@ -116,8 +118,8 @@ Switches to the specified vault for subsequent operations.
 
 #### `secret vault remove <name> [--force]` / `secret vault rm` ⚠️ 🛑
 
-**DANGER**: Permanently removes a vault and all its secrets. It first asks for
+**DANGER**: Permanently removes a vault and all its secrets. It first asks
-confirmation, naming the vault and how many secrets it holds (see
+for confirmation, naming the vault and how many secrets it holds (see
 [Confirmation Before Removal](#confirmation-before-removal)). The last vault
 cannot be removed. Removing the current vault makes another vault the current
 one.
@@ -130,65 +132,58 @@ one.
 #### `secret add <secret-name> [--force]`
 
 Adds a secret to the current vault. Reads the secret value from stdin.
-
 - `--force, -f`: Overwrite existing secret
 
-**Secret Name Format:** only ASCII letters, digits, `.`, `-`, `_` and `/` are
+**Secret Name Format:** only ASCII letters, digits, `.`, `-`, `_` and `/`
-allowed, and a name must not be empty, start with `.` or `/`, end with `/`,
+are allowed, and a name must not be empty, start with `.` or `/`, end with
-contain `//`, or have `..` as a path segment.
+`/`, contain `//`, or have `..` as a path segment.
-
 - Forward slashes (`/`) are converted to percent signs (`%`) for storage
 - Examples: `database/password`, `api.key`, `ssh_private_key`
 
 #### `secret get <secret-name> [--version <version>]`
 
 Retrieves and outputs a secret value to stdout.
-
 - `--version, -v`: Get a specific version (default: current)
 
 #### `secret list [filter] [--json]` / `secret ls`
 
-Lists all secrets in the current vault. Optional filter for substring matching.
+Lists all secrets in the current vault. Optional filter for substring
+matching.
 
 #### `secret remove <secret-name> [--force]` / `secret rm` ⚠️ 🛑
 
-**DANGER**: Permanently removes a secret and ALL its versions. It first asks for
+**DANGER**: Permanently removes a secret and ALL its versions. It first asks
-confirmation, naming the secret, its vault and how many versions it has (see
+for confirmation, naming the secret, its vault and how many versions it has
-[Confirmation Before Removal](#confirmation-before-removal)).
+(see [Confirmation Before Removal](#confirmation-before-removal)).
-
 - `--force, -f`: Remove without asking
 - **NO RECOVERY**: Once removed, the secret cannot be recovered
-- **ALL VERSIONS DELETED**: Every version of the secret will be permanently
+- **ALL VERSIONS DELETED**: Every version of the secret will be permanently deleted
-  deleted
 
 #### `secret move <source> <destination>` / `secret mv` / `secret rename`
 
 Moves or renames a secret within the current vault.
-
 - Fails if the destination already exists
-- Fails if the destination is the source under another name, such as `foo` for
+- Fails if the destination is the source under another name, such as `foo`
-  `Foo` on a case-insensitive filesystem (the macOS default); there, to change
+  for `Foo` on a case-insensitive filesystem (the macOS default); there, to
-  only the case of a name, move the secret to a third name first
+  change only the case of a name, move the secret to a third name first
 - Preserves all versions and metadata
 
 ### Version Management
 
 #### `secret version list <secret-name>` / `secret version ls`
 
-Lists all versions of a secret showing creation time, status, and validity
+Lists all versions of a secret showing creation time, status, and validity period.
-period.
 
 #### `secret version promote <secret-name> <version>`
 
-Promotes a specific version to current by updating the symlink. Does not modify
+Promotes a specific version to current by updating the symlink. Does not
-any timestamps, allowing for rollback scenarios.
+modify any timestamps, allowing for rollback scenarios.
 
 #### `secret version remove <secret-name> <version> [--force]` / `secret version rm` ⚠️ 🛑
 
 **DANGER**: Permanently removes a specific version of a secret. It first asks
 for confirmation, naming the version, the secret and its vault (see
 [Confirmation Before Removal](#confirmation-before-removal)).
-
 - `--force, -f`: Remove without asking
 - **NO RECOVERY**: Once removed, this version cannot be recovered
 - Cannot remove the current version (must promote another version first)
@@ -202,7 +197,6 @@ Generates a cryptographically secure BIP39 mnemonic phrase.
 #### `secret generate secret <name> [--length=16] [--type=base58] [--force]`
 
 Generates and stores a random secret.
-
 - `--length, -l`: Length of generated secret (default: 16)
 - `--type, -t`: Type of secret (`base58`, `alnum`)
 - `--force, -f`: Overwrite existing secret
@@ -218,19 +212,16 @@ Lists all unlockers in the current vault with their metadata.
 Creates a new unlocker of the specified type:
 
 **Types:**
-
 - `passphrase`: Traditional passphrase-protected unlocker
 - `pgp`: Uses an existing GPG key for encryption/decryption
 - `keychain`: macOS Keychain integration (macOS only)
 - `secure-enclave`: Hardware-backed Secure Enclave protection (macOS only)
 
 **Options:**
+- `--keyid <id>`: GPG key ID (optional for PGP type, uses default key if not specified)
 
-- `--keyid <id>`: GPG key ID (optional for PGP type, uses default key if not
+A vault has one passphrase unlocker: adding one replaces the one the vault
-  specified)
+has, which is removed only once the new one is the current unlocker.
-
-A vault has one passphrase unlocker: adding one replaces the one the vault has,
-which is removed only once the new one is the current unlocker.
 
 #### `secret unlocker remove <unlocker-id> [--force]` / `secret unlocker rm` ⚠️ 🛑
 
@@ -239,9 +230,9 @@ naming the unlocker and its vault and saying whether it is the vault's last
 unlocker; for the last one it says how many secrets the vault holds and warns
 that the vault then opens only with its mnemonic (see
 [Confirmation Before Removal](#confirmation-before-removal)). An unlocker
-directory that `secret unlocker list` skips with a warning, because its metadata
+directory that `secret unlocker list` skips with a warning, because its
-cannot be read or parsed, is removed by the directory name the warning gives.
+metadata cannot be read or parsed, is removed by the directory name the
-
+warning gives.
 - `--force, -f`: Remove without asking, even the last unlocker
 - **CRITICAL WARNING**: Without unlockers and without your mnemonic phrase,
   vault data will be PERMANENTLY INACCESSIBLE
@@ -256,8 +247,7 @@ Selects an unlocker as the current default for operations.
 
 #### `secret import <secret-name> --source <filename>`
 
-Imports a secret from a file and stores it in the current vault under the given
+Imports a secret from a file and stores it in the current vault under the given name.
-name.
 
 #### `secret vault import [vault-name]`
 
@@ -267,8 +257,7 @@ Imports a mnemonic phrase into the specified vault (defaults to "default").
 
 #### `secret encrypt <secret-name> [--input=file] [--output=file]`
 
-Encrypts data using an Age key stored as a secret. If the secret doesn't exist,
+Encrypts data using an Age key stored as a secret. If the secret doesn't exist, generates a new Age key.
-generates a new Age key.
 
 #### `secret decrypt <secret-name> [--input=file] [--output=file]`
 
@@ -313,43 +302,37 @@ Decrypts data using an Age key stored as a secret.
 ### Key Management and Encryption Flow
 
 #### 1: Long-term Keys
-
+- **Source**: Derived from BIP39 mnemonic phrases using hierarchical deterministic (HD) key derivation
-- **Source**: Derived from BIP39 mnemonic phrases using hierarchical
-  deterministic (HD) key derivation
 - **Purpose**: Master keys for each vault, used to encrypt secret-specific keys
-- **Storage**: Public key stored as `pub.age`, private key encrypted by
+- **Storage**: Public key stored as `pub.age`, private key encrypted by unlockers
-  unlockers
 
 #### 2: Unlockers
 
 Unlockers provide different authentication methods to access the long-term keys:
 
 1. **Passphrase Unlockers**:
-    - Encrypted with user-provided passphrase
+   - Encrypted with user-provided passphrase
-    - Stored as encrypted Age keys
+   - Stored as encrypted Age keys
-    - Cross-platform compatible
+   - Cross-platform compatible
 
 2. **PGP Unlockers**:
-    - Uses existing GPG key infrastructure
+   - Uses existing GPG key infrastructure
-    - Leverages existing key management workflows
+   - Leverages existing key management workflows
-    - Strong authentication through GPG
+   - Strong authentication through GPG
 
 3. **Keychain Unlockers** (macOS only):
-    - Stores unlock keys in macOS Keychain
+   - Stores unlock keys in macOS Keychain
-    - Protected by system authentication (Touch ID, password)
+   - Protected by system authentication (Touch ID, password)
-    - Automatic unlocking when Keychain is unlocked
+   - Automatic unlocking when Keychain is unlocked
-    - Cross-application integration
+   - Cross-application integration
 
 4. **Secure Enclave Unlockers** (macOS):
-    - Hardware-backed key storage using Apple Secure Enclave
+   - Hardware-backed key storage using Apple Secure Enclave
-    - Uses `sc_auth` / CryptoTokenKit for SE key management (no Apple Developer
+   - Uses `sc_auth` / CryptoTokenKit for SE key management (no Apple Developer Program required)
-      Program required)
+   - ECIES encryption: vault long-term key encrypted directly by SE hardware
-    - ECIES encryption: vault long-term key encrypted directly by SE hardware
+   - Protected by biometric authentication (Touch ID) or system password
-    - Protected by biometric authentication (Touch ID) or system password
 
-Each vault maintains its own set of unlockers and one long-term key. The
+Each vault maintains its own set of unlockers and one long-term key. The long-term key is encrypted to each unlocker, allowing any authorized unlocker to access vault secrets.
-long-term key is encrypted to each unlocker, allowing any authorized unlocker to
-access vault secrets.
 
 #### 3: Secret-specific Keys
 
@@ -369,19 +352,18 @@ they hold. Other processes running as the same user can read a process's
 environment (on Linux, from `/proc/<pid>/environ`). Every child process of the
 shell or script that sets them inherits them, `gpg` included. Set on a command
 line or in a CI job, they end up in shell history and CI logs. `secret` unsets
-each one as soon as it has read it, so that the programs it runs itself, such as
+each one as soon as it has read it, so that the programs it runs itself, such
-`gpg`, do not inherit it, but that erases nothing: the environment the process
+as `gpg`, do not inherit it, but that erases nothing: the environment the
-started with, and its memory, still hold the value. The interactive prompt,
+process started with, and its memory, still hold the value. The interactive
-which every command except `secret vault import` offers when the variable is not
+prompt, which every command except `secret vault import` offers when the
-set, is the safer default; `secret vault import` has no prompt and needs both
+variable is not set, is the safer default; `secret vault import` has no prompt
-variables.
+and needs both variables.
 
 ## Security Features
 
 ### Encryption
 
-- Uses the [age encryption library](https://age-encryption.org/) with X25519
+- Uses the [age encryption library](https://age-encryption.org/) with X25519 keys
-  keys
 - All private keys are encrypted at rest
 - No plaintext secrets stored on disk
 
@@ -400,8 +382,7 @@ variables.
 
 - Hardware token support via PGP/GPG integration
 - macOS Keychain integration for system-level security
-- Secure Enclave integration for hardware-backed key protection (macOS, via
+- Secure Enclave integration for hardware-backed key protection (macOS, via `sc_auth` / CryptoTokenKit)
-  `sc_auth` / CryptoTokenKit)
 
 ## Examples
 
@@ -450,7 +431,6 @@ secret vault remove personal --force
 ```
 
 ### Advanced Authentication
-
 ```bash
 # Add multiple unlock methods
 secret unlocker add passphrase              # Password-based
@@ -497,27 +477,21 @@ secret decrypt encryption/mykey --input document.txt.age --output document.txt
 ## Technical Details
 
 ### Cryptographic Primitives
-
 - **Key Derivation**: BIP32/BIP39 hierarchical deterministic key derivation
 - **Encryption**: Age (X25519 + ChaCha20-Poly1305)
 - **Authentication**: Poly1305 MAC
 - **Hashing**: Double SHA-256 for public key identification
 
 ### File Formats
-
 - **age Files**: Standard age encryption format (.age extension)
 - **Metadata**: Unencrypted JSON format with timestamps and type information
-- **Vault Metadata**: JSON containing vault name, creation time, derivation
+- **Vault Metadata**: JSON containing vault name, creation time, derivation index, and public key hash
-  index, and public key hash
 
 ### Vault Management
 
-- **Derivation Index**: Each vault uses a unique derivation index from the
+- **Derivation Index**: Each vault uses a unique derivation index from the mnemonic, and thus a unique key pair
-  mnemonic, and thus a unique key pair
+- **Public Key Hash**: Double SHA-256 hash of the index-0 public key identifies vaults from the same mnemonic
-- **Public Key Hash**: Double SHA-256 hash of the index-0 public key identifies
+- **Automatic Key Derivation**: When creating vaults with a mnemonic, keys are automatically derived
-  vaults from the same mnemonic
-- **Automatic Key Derivation**: When creating vaults with a mnemonic, keys are
-  automatically derived
 
 ### Cross-Platform Support
 
@@ -553,7 +527,6 @@ to add or use them.
 ## Development
 
 ### Building
-
 ```bash
 make build    # Build binary
 make test     # Run tests
@@ -561,9 +534,7 @@ make lint     # Run linter
 ```
 
 ### Testing
-
 The project includes comprehensive tests:
-
 ```bash
 make test     # Run all tests
 go test ./... # Unit tests
@@ -575,68 +546,61 @@ go test -tags=integration -v ./internal/cli  # Integration tests
 This repository adheres to the
 [Scripts to Rule Them All](https://github.com/github/scripts-to-rule-them-all)
 standard: normalized scripts in `script/` are the entrypoints for the
-development workflow, and the Makefile targets are thin shims that call them. We
+development workflow, and the Makefile targets are thin shims that call
-provide:
+them. We provide:
 
-- `script/bootstrap` — install all dependencies (Go, Go module download),
+- `script/bootstrap` — install all dependencies (Go, Go module
-  idempotently; golangci-lint is not installed, it runs in docker
+  download), idempotently; golangci-lint is not installed, it runs in
+  docker
 - `script/setup` — make a fresh clone ready for development: runs
   `script/bootstrap`, then `script/install-precommit`
-- `script/projectname` — output the project name (`secret`); used by other
+- `script/projectname` — output the project name (`secret`); used by
-  scripts such as `script/docker`
+  other scripts such as `script/docker`
-- `script/build` — build the `secret` binary into the repo root, stamping the
+- `script/build` — build the `secret` binary into the repo root, stamping
-  version (`VERSION` from the environment, else `git describe`) and the git
+  the version (`VERSION` from the environment, else `git describe`) and
-  commit
+  the git commit
-- `script/test` — run `go vet` and the test suite (verbose rerun on failure)
+- `script/test` — run `go vet` and the test suite (verbose rerun on
-- `script/lint` — run `golangci-lint` in docker only: builds `Dockerfile.lint`,
+  failure)
-  where the linter is a build step that runs on every call, also on an unchanged
+- `script/lint` — run `golangci-lint` in docker only: builds
-  tree
+  `Dockerfile.lint`, where the linter is a build step that runs on every
-- `script/lint-darwin` — run `go vet` and `golangci-lint` in docker on the code
+  call, also on an unchanged tree
-  as a macOS build compiles it (`GOOS=darwin`), which a Linux build never
+- `script/lint-darwin` — run `go vet` and `golangci-lint` in docker on
-  compiles; cgo is off, so the keychain unlocker's calls into the keychain
+  the code as a macOS build compiles it (`GOOS=darwin`), which a Linux
-  (`internal/secret/keychainunlocker_cgo.go`, and `keychainunlocker_test.go`)
+  build never compiles; cgo is off, so the keychain unlocker's calls into
-  and the Secure Enclave bindings (`internal/macse`) are not checked
+  the keychain (`internal/secret/keychainunlocker_cgo.go`, and
+  `keychainunlocker_test.go`) and the Secure Enclave bindings
+  (`internal/macse`) are not checked
 - `script/fmt` — format all Go code (writes)
 - `script/fmt-check` — check formatting without writing
-- `script/check` — run `script/test`, `script/lint`, `script/lint-darwin`, and
+- `script/check` — run `script/test`, `script/lint`,
-  `script/fmt-check`
+  `script/lint-darwin`, and `script/fmt-check`
 - `script/docker` — build the Docker image tagged with the project name
-- `script/cibuild` — CI entrypoint: `docker build --ulimit memlock=-1:-1 .`
+- `script/cibuild` — CI entrypoint: `docker build --ulimit
-  (memguard needs mlock; the Dockerfile runs the checks), with a new
+  memlock=-1:-1 .` (memguard needs mlock; the Dockerfile runs the
-  `CHECK_EPOCH` build argument on every run so the checks run again on an
+  checks), with a new `CHECK_EPOCH` build argument on every run so the
-  unchanged tree
+  checks run again on an unchanged tree
-- `script/precommit` — pre-commit checks: `go mod tidy` verification, then
+- `script/precommit` — pre-commit checks: `go mod tidy` verification,
-  `script/check`
+  then `script/check`
-- `script/install-precommit` — install the git pre-commit hook that runs
+- `script/install-precommit` — install the git pre-commit hook that
-  `script/precommit`
+  runs `script/precommit`
 
 ## Features
 
-- **Multiple Authentication Methods**: Supports passphrase, PGP, macOS Keychain,
+- **Multiple Authentication Methods**: Supports passphrase, PGP, macOS Keychain, and Secure Enclave unlockers
-  and Secure Enclave unlockers
 - **Vault Isolation**: Complete separation between different vaults
 - **Per-Secret Encryption**: Each secret has its own encryption key
 - **BIP39 Mnemonic Support**: Keyless operation using mnemonic phrases
 - **Cross-Platform**: Works on macOS, Linux, and other Unix-like systems
 
-## TODO
+# Author
 
-Open work is tracked on the
+Made with love and lots of expensive SOTA AI by
-[issue tracker](https://git.eeqj.de/sneak/secret/issues), which is
+[sneak](https://sneak.berlin) in Berlin in the summer of 2025.
-authoritative. The work to be done before 1.0 is the
-[`1.0.0` milestone](https://git.eeqj.de/sneak/secret/milestone/12). `TODO.md`
-records the steps completed so far.
 
-## License
+Released as a free software gift to the world, no strings attached, under
-
+the [WTFPL](https://www.wtfpl.net/) license.
-Released as a free software gift to the world, no strings attached, under the
-[WTFPL](https://www.wtfpl.net/) license; see [`LICENSE`](LICENSE).
-
-## Author
-
-Made with love and lots of expensive SOTA AI by [@sneak](https://sneak.berlin)
-in Berlin in the summer of 2025.
 
 Contact: [sneak@sneak.berlin](mailto:sneak@sneak.berlin)
 
 [https://keys.openpgp.org/vks/v1/by-fingerprint/5539AD00DE4C42F3AFE11575052443F4DF2A55C2](https://keys.openpgp.org/vks/v1/by-fingerprint/5539AD00DE4C42F3AFE11575052443F4DF2A55C2)
+

TODO.md

@@ -1,20 +1,27 @@
 # Workflow
 
-- branch from `next`
+* branch (from `main`)
-- do the Next Step: the next open issue in the `1.0.0` milestone
+* do the work in Next Step
-- log it at the top of Completed Steps
+* move Next Step to the top of Completed Steps
-- commit (`TODO.md` changes in the same commit as the work)
+* move the top item of Future Steps into Next Step
-- push, and open a PR against `next`
+* commit (`TODO.md` changes in the same commit as the work)
+* merge to `main` if the branch is not protected, otherwise open a PR
+* push
 
 # Status
 
-pre-1.0. No git tags. Open work is tracked on the issue tracker, which is
+pre-1.0. No git tags. TODO.md carries open 1.0 security blockers. Work in
-authoritative.
+flight on branch secure-enclave-unlocker (clean tree as of 2026-07-06).
 
 # Next Step
 
-Take the next open issue in the `1.0.0` milestone:
+Bring the repo into policy compliance in one commit:
-https://git.eeqj.de/sneak/secret/milestone/12
+
+- Add fmt-check and hooks targets to the Makefile (test/lint/fmt/check/
+  docker already exist).
+- Add REPO_POLICIES.md and .editorconfig.
+- Add .gitea/workflows/check.yml running make check.
+- Verify Dockerfile base images are pinned by sha256.
 
 # Completed Steps
 
@@ -24,31 +31,12 @@ https://git.eeqj.de/sneak/secret/milestone/12
   `CreateSecureEnclaveUnlocker` gets the long-term key before it creates the
   Secure Enclave key, so that a wrong passphrase creates none, and deletes the
   key again if encrypting with it or writing the unlocker then fails.
-  `macse.CreateKey` finds the new key's hash right after `sc_auth` creates
-  it, and fails with an error naming the key's label if it cannot; it deletes
-  the key again if getting its public key then fails. The Objective-C was only
-  read, never compiled or run, and so was `macse_darwin.go`, which is cgo only.
   `CreateKeychainUnlocker` writes all of the unlocker's files, the metadata
   among them, before it stores the item in the keychain, and deletes the item
   again if moving the unlocker into place then fails. A failure to delete is
   reported along with the first error. The tests of this run only on macOS:
   the Secure Enclave one in a build with cgo on a Mac with a Secure Enclave,
   the keychain one in a build with cgo.
-- 2026-10-04: What a command killed part-way left under a `.tmp-` name
-  (https://git.eeqj.de/sneak/secret/issues/75), the temporary directories
-  of `secret.TempDirFor` and the temporary files of
-  `secret.WriteFileAtomic`, encrypted keys included, is deleted by the next
-  command that takes the state directory lock. Before, it stayed until
-  deleted by hand. A command writes `finished` into the lock file just
-  before it releases the lock; the next one to take the lock searches only
-  when it does not find that, so after a command that finished nothing is
-  searched, however many secrets and versions there are. The search looks
-  in the state directory, each vault, each secret and each version, the
-  only directories those helpers make them in. A command that only reads
-  takes no lock and deletes nothing. A failure to delete is warned about
-  and the command goes on. An unlocker directory with no metadata file was
-  already removed by `secret unlocker remove` given its directory name; a
-  test now shows it.
 - 2026-10-04: An age identity's private key goes into a locked buffer
   through `secret.IdentityToLockedBuffer` everywhere
   (https://git.eeqj.de/sneak/secret/issues/38): the vault's long-term key
@@ -259,10 +247,15 @@ https://git.eeqj.de/sneak/secret/milestone/12
   cross-vault copies are built in a temporary directory and renamed
   into place, and removals rename out of the way first, so a version
   or secret is never half-added and never half-removed. An
-  interrupted command can still leave, from `init` or `vault create`
+  interrupted command can still leave:
-  killed after the passphrase prompt but before the unlocker is
+    - from `init` or `vault create` killed after the passphrase prompt
-  written, a vault with no unlocker, which `vault create` has already
+      but before the unlocker is written, a vault with no unlocker,
-  made the current vault.
+      which `vault create` has already made the current vault;
+    - data under a `.tmp-` name in the state directory: a secret,
+      version or unlocker being added, or the secret, version, unlocker
+      or vault being removed, encrypted keys included. Nothing deletes
+      it; it must be deleted by hand
+      (https://git.eeqj.de/sneak/secret/issues/75).
 - 2026-10-03: The checks run before changing a vault now stop with an
   error naming the path and cause when they cannot read what they
   inspect, instead of reading the failure as "nothing there": the
@@ -318,15 +311,8 @@ https://git.eeqj.de/sneak/secret/milestone/12
   `findUnlockerIDByMetadata` now returns an error so `unlocker list`
   skips an unreadable `unlockers.d` entry with a warning instead of
   emitting a fabricated fallback ID.
-- 2026-08-07: Added `.editorconfig`
-  (https://git.eeqj.de/sneak/secret/issues/27).
 - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
   Makefile shims, README Entrypoints section
-- 2026-07-07: Added `REPO_POLICIES.md` and the `make hooks` target;
-  `.gitea/workflows/check.yml` now runs `script/cibuild`.
-- 2026-03-30: Added the `make fmt-check` target and
-  `.gitea/workflows/check.yml`, which runs `docker build` on every push; the
-  `Dockerfile` base images are pinned by sha256.
 - 2026-03-11: Secure Enclave unlocker for hardware-backed secret
   protection, plus review fixes (stub panics, derivation index, tests,
   README) on branch secure-enclave-unlocker.
@@ -348,6 +334,8 @@ https://git.eeqj.de/sneak/secret/milestone/12
 
 # Future Steps
 
+- Compliance (after Next Step lands): keep main green under the new
+  .gitea workflow; run make check before every merge.
 - Implement version-number shell completion for the second arg of
   `secret version promote` and `secret version rm`
   (`internal/cli/version.go`; was an in-code TODO removed for godox).
@@ -362,18 +350,34 @@ https://git.eeqj.de/sneak/secret/milestone/12
   never run on them, so it would likely find more there than the line
   lengths. No macOS test runs in CI. A macOS runner would cover all of it
   (asked on https://git.eeqj.de/sneak/secret/issues/50).
+- Merge secure-enclave-unlocker to main once review is done.
 - 1.0 critical security blockers (from repo TODO.md):
-    - Memory security: age writes an identity's private key out as a string in
+    - Command injection: GPG key IDs passed unescaped to exec.Command
-      ordinary memory, and the copies it makes on the way stay there
+      (pgpunlocker.go:323-327); data.String() passed unescaped to the
-      (`secret.IdentityToLockedBuffer` overwrites only the string itself).
+      security command (keychainunlocker.go:472-476).
+    - Memory security: age writes an identity's private key out as a
+      string in ordinary memory, and the copies it makes on the way stay
+      there (`secret.IdentityToLockedBuffer` overwrites only the string
+      itself); private keys exposed via buffer.Bytes() to GPGEncryptFunc
+      and EncryptWithPassphrase.
+    - Input validation: no maximum secret size (DoS).
+    - Timing attacks: bytes.Equal passphrase compare (cli/init.go:
+      209-216); non-constant-time public key compare (vault.go:95-100).
+- High priority:
+    - Secure temporary file handling and cleanup.
+    - Initialize a default unlock key at vault creation.
+    - Add secret rm and vault deletion commands.
 - Medium priority:
     - Standardize error messages; stop leaking internals.
     - Graceful handling of corrupted or missing key files with recovery
       suggestions.
+    - Validate GPG key existence before creating PGP unlock keys.
     - Split oversized CLI functions.
+    - mlock/munlock for sensitive allocations.
 - Cleanups: read statedir from environment or default instead of
   passing it around.
-- Enhancements: help examples, colored output, --quiet flag, name suggestions on
+- Enhancements: help examples, shell completion, colored output,
-  miss, audit logging, hardware integration tests (Keychain, GPG), naming
+  --quiet flag, name suggestions on miss, audit logging, hardware
-  consistency, vault export/import, batch operations, search, secret metadata
+  integration tests (Keychain, GPG), naming consistency, vault
+  export/import, batch operations, search, secret metadata
   (descriptions, tags).

internal/cli/leftovers_test.go

@@ -1,74 +0,0 @@
-package cli_test
-
-import (
-	"io"
-	"testing"
-
-	"git.eeqj.de/sneak/secret/internal/cli"
-	"git.eeqj.de/sneak/secret/internal/secret"
-	"git.eeqj.de/sneak/secret/internal/vault"
-	"github.com/spf13/afero"
-	"github.com/spf13/cobra"
-	"github.com/stretchr/testify/require"
-)
-
-// TestLeftoversRemovedByNextChangingCommand is a regression test for
-// https://git.eeqj.de/sneak/secret/issues/75. It plants what a command
-// killed part-way leaves in each directory where secret.TempDirFor and
-// secret.WriteFileAtomic make temporary entries: a temporary directory
-// holding a vault, secret, unlocker or version being added or removed, and
-// a temporary file beside a file being replaced. `secret list` must leave
-// them all, and the next command that takes the state directory lock, here
-// `secret vault select` of the vault already current, must delete exactly
-// them: a vault named like a temporary directory stays. The copy has no
-// lock file yet, so that command, as after a killed one, finds no mark that
-// the last holder of the lock finished.
-func TestLeftoversRemovedByNextChangingCommand(t *testing.T) {
-	t.Parallel()
-
-	fs := newTwoVaultFs(t)
-
-	_, err := vault.CreateVault(fs, testStateDir, ".tmp-0", nil)
-	require.NoError(t, err)
-	require.NoError(t, vault.SelectVault(fs, testStateDir, "default"))
-
-	before := snapshotStateDir(t, fs)
-
-	vaultDir := testStateDir + "/vaults.d/default"
-	secretDir := vaultDir + "/secrets.d/x"
-
-	versions, err := secret.ListVersions(fs, secretDir)
-	require.NoError(t, err)
-	require.Len(t, versions, 1)
-
-	for _, dir := range []string{
-		testStateDir + "/.tmp-1/default",
-		vaultDir + "/.tmp-2/x",
-		secretDir + "/.tmp-3/" + testVersion,
-	} {
-		require.NoError(t, fs.MkdirAll(dir, secret.DirPerms))
-		require.NoError(t, afero.WriteFile(fs, dir+"/value.age",
-			[]byte("encrypted"), secret.FilePerms))
-	}
-
-	for _, file := range []string{
-		testStateDir + "/.currentvault.tmp-4",
-		vaultDir + "/.current-unlocker.tmp-5",
-		secretDir + "/.current.tmp-6",
-		secretDir + "/versions/" + versions[0] + "/.metadata.age.tmp-7",
-	} {
-		require.NoError(t, afero.WriteFile(fs, file,
-			[]byte("partial"), secret.FilePerms))
-	}
-
-	planted := snapshotStateDir(t, fs)
-	c := cli.NewCLIInstanceWithStateDir(fs, testStateDir)
-	cmd := &cobra.Command{}
-	cmd.SetOut(io.Discard)
-
-	require.NoError(t, c.ListSecrets(cmd, false, false, ""))
-	require.Equal(t, planted, snapshotStateDir(t, fs))
-
-	require.NoError(t, c.SelectVault(cmd, "default"))
-	require.Equal(t, before, snapshotStateDir(t, fs))
-}

internal/cli/lock_test.go

@@ -362,6 +362,7 @@ func requireWaitsForLock(
 
 	fs := afero.NewMemMapFs()
 	olderVersion, unlockerID := setupEveryCommand(t, fs, withUnlocker)
+	before := stateDirModTimes(t, fs)
 
 	release, err := vault.LockStateDir(fs, testStateDir)
 	require.NoError(t, err)
@@ -371,9 +372,6 @@ func requireWaitsForLock(
 	release = sync.OnceFunc(release)
 	defer release()
 
-	// Taken only now, since taking the lock writes the lock file.
-	before := stateDirModTimes(t, fs)
-
 	unlockPassphrase := memguard.NewBufferFromBytes([]byte(testPassphrase))
 	defer unlockPassphrase.Destroy()
 

internal/cli/path_traversal_test.go

@@ -90,8 +90,6 @@ func newTwoVaultFs(t *testing.T) afero.Fs {
 // snapshotStateDir maps every file under the state directory to its
 // contents, and every directory, written with a trailing "/", to "". Two
 // snapshots are equal only if nothing in it was added, removed or changed.
-// The lock file, which every command that takes the lock writes, is left
-// out.
 func snapshotStateDir(t *testing.T, fs afero.Fs) map[string]string {
 	t.Helper()
 
@@ -104,10 +102,6 @@ func snapshotStateDir(t *testing.T, fs afero.Fs) map[string]string {
 			return err
 		}
 
-		if path == testStateDir+"/lock" {
-			return nil
-		}
-
 		if info.IsDir() {
 			tree[path+"/"] = ""
 

internal/cli/unlockers_corrupt_test.go

@@ -4,10 +4,9 @@
 // by its ID. These tests give the first unlocker, which sorts before the
 // one the commands act on, metadata that is not JSON, and check that the
 // commands step past it, and that it can itself be removed by its
-// directory name, which `secret unlocker list` names in its warning, as can
+// directory name, which `secret unlocker list` names in its warning. A
-// one with no metadata file. A last test checks that an unlocker whose
+// last test checks that an unlocker whose metadata file cannot be read
-// metadata file cannot be read counts as the last unlocker when it is
+// counts as the last unlocker when it is removed by its directory name.
-// removed by its directory name.
 
 //nolint:testpackage // white-box test of unexported internals
 package cli
@@ -107,34 +106,6 @@ func TestUnlockerRemoveWithCorruptUnlocker(t *testing.T) {
 	}
 }
 
-// TestUnlockerRemoveWithoutMetadata asserts that a partial unlocker
-// directory, one with no metadata file, removed by its directory name from
-// a vault with secrets, does not count as the vault's last unlocker, since
-// it cannot unlock the vault, so the question says it is not. It is
-// removed once the user confirms.
-func TestUnlockerRemoveWithoutMetadata(t *testing.T) {
-	t.Parallel()
-
-	fs := newListTestVault(t, 2)
-	vaultDir := testVaultDir(listTestVaultName)
-	unlockersDir := filepath.Join(vaultDir, listTestUnlockersDirName)
-
-	require.NoError(t, fs.Remove(filepath.Join(
-		unlockersDir, listTestUnlockerDirOne, listTestMetadataFileName)))
-	writeTestSecret(t, fs, vaultDir)
-
-	instance, cmd := newTestInstance(fs)
-
-	found, err := instance.findUnlockerToRemove(listTestUnlockerDirOne)
-	require.NoError(t, err)
-	assert.False(t, found.last)
-	assert.Contains(t, found.question, "not the vault's last unlocker")
-
-	instance.terminal = strings.NewReader("y\n")
-	require.NoError(t, instance.UnlockersRemove(listTestUnlockerDirOne, false, cmd))
-	assertDirEntries(t, fs, unlockersDir, listTestUnlockerDirTwo)
-}
-
 // TestUnlockerRemoveWithUnreadableMetadata asserts that the only unlocker
 // of a vault with secrets, removed by its directory name when its metadata
 // file cannot be checked for or read, counts as the vault's last unlocker,

internal/macse/macse_darwin.go

@@ -15,7 +15,6 @@ package macse
 import "C"
 
 import (
-	"errors"
 	"fmt"
 	"unsafe"
 )
@@ -40,9 +39,10 @@ const (
 
 // CreateKey creates a new P-256 non-exportable key in the Secure Enclave via sc_auth.
 // Returns the uncompressed public key bytes (65 bytes) and the identity hash
-// (for deletion). If getting the public key fails, CreateKey deletes the key
+// (for deletion).
-// again; a failure to delete is returned along with the first error.
 func CreateKey(label string) (publicKey []byte, hash string, err error) {
+	pubKeyBuf := make([]C.uint8_t, p256UncompressedKeySize)
+	pubKeyLen := C.int(p256UncompressedKeySize)
 	var hashBuf [hashBufferSize]C.char
 	var errBuf [errorBufferSize]C.char
 
@@ -50,6 +50,7 @@ func CreateKey(label string) (publicKey []byte, hash string, err error) {
 	defer C.free(unsafe.Pointer(cLabel)) //nolint:nlreturn // CGo free pattern
 
 	result := C.se_create_key(cLabel,
+		&pubKeyBuf[0], &pubKeyLen,
 		&hashBuf[0], C.int(hashBufferSize),
 		&errBuf[0], C.int(errorBufferSize))
 
@@ -57,29 +58,9 @@ func CreateKey(label string) (publicKey []byte, hash string, err error) {
 		return nil, "", fmt.Errorf("secure enclave: %s", C.GoString(&errBuf[0]))
 	}
 
-	h := C.GoString(&hashBuf[0])
-
-	pubKeyBuf := make([]C.uint8_t, p256UncompressedKeySize)
-	pubKeyLen := C.int(p256UncompressedKeySize)
-
-	result = C.se_copy_public_key(cLabel,
-		&pubKeyBuf[0], &pubKeyLen,
-		&errBuf[0], C.int(errorBufferSize))
-
-	if result != 0 {
-		err = fmt.Errorf("secure enclave: %s", C.GoString(&errBuf[0]))
-
-		deleteErr := DeleteKey(h)
-		if deleteErr != nil {
-			err = errors.Join(err,
-				fmt.Errorf("failed to delete key %s: %w", label, deleteErr))
-		}
-
-		return nil, "", err
-	}
-
 	//nolint:nlreturn // CGo result extraction
 	pk := C.GoBytes(unsafe.Pointer(&pubKeyBuf[0]), pubKeyLen)
+	h := C.GoString(&hashBuf[0])
 
 	return pk, h, nil
 }

internal/macse/secure_enclave.h

@@ -5,30 +5,20 @@
 
 #include <stdint.h>
 
-// se_create_key creates a new P-256 key in the Secure Enclave via sc_auth and
+// se_create_key creates a new P-256 key in the Secure Enclave via sc_auth.
-// finds its identity hash. If the hash cannot be found, the key exists but
-// se_create_key fails, with an error naming the label.
 // label: unique identifier for the CTK identity (UTF-8 C string)
+// pub_key_out: output buffer for the uncompressed public key (65 bytes for P-256)
+// pub_key_len: on input, size of pub_key_out; on output, actual size written
 // hash_out: output buffer for the identity hash (for deletion)
 // hash_out_len: size of hash_out buffer
 // error_out: output buffer for error message
 // error_out_len: size of error_out buffer
 // Returns 0 on success, -1 on failure.
 int se_create_key(const char *label,
+                  uint8_t *pub_key_out, int *pub_key_len,
                   char *hash_out, int hash_out_len,
                   char *error_out, int error_out_len);
 
-// se_copy_public_key copies the public key of a CTK identity.
-// label: label of the CTK identity
-// pub_key_out: output buffer for the uncompressed public key (65 bytes for P-256)
-// pub_key_len: on input, size of pub_key_out; on output, actual size written
-// error_out: output buffer for error message
-// error_out_len: size of error_out buffer
-// Returns 0 on success, -1 on failure.
-int se_copy_public_key(const char *label,
-                       uint8_t *pub_key_out, int *pub_key_len,
-                       char *error_out, int error_out_len);
-
 // se_encrypt encrypts data using the SE-backed public key (ECIES).
 // label: label of the CTK identity whose public key to use
 // plaintext: data to encrypt

internal/macse/secure_enclave.m

@@ -47,6 +47,7 @@ static SecKeyRef lookup_ctk_private_key(const char *label, char *error_out, int
 }
 
 int se_create_key(const char *label,
+                  uint8_t *pub_key_out, int *pub_key_len,
                   char *hash_out, int hash_out_len,
                   char *error_out, int error_out_len) {
     @autoreleasepool {
@@ -86,56 +87,7 @@ int se_create_key(const char *label,
             return -1;
         }
 
-        // Get the identity hash, which deleting the key needs, by parsing
+        // Retrieve the public key from the created identity
-        // sc_auth list output
-        hash_out[0] = '\0';
-        NSTask *listTask = [[NSTask alloc] init];
-        listTask.executableURL = [NSURL fileURLWithPath:@"/usr/sbin/sc_auth"];
-        listTask.arguments = @[@"list-ctk-identities"];
-
-        NSPipe *listPipe = [NSPipe pipe];
-        listTask.standardOutput = listPipe;
-        listTask.standardError = [NSPipe pipe];
-
-        if ([listTask launchAndReturnError:&nsError]) {
-            [listTask waitUntilExit];
-            NSData *listData = [listPipe.fileHandleForReading readDataToEndOfFile];
-            NSString *listStr = [[NSString alloc] initWithData:listData
-                                                      encoding:NSUTF8StringEncoding];
-
-            for (NSString *line in [listStr componentsSeparatedByString:@"\n"]) {
-                if ([line containsString:labelStr]) {
-                    NSMutableArray *tokens = [NSMutableArray array];
-                    for (NSString *part in [line componentsSeparatedByCharactersInSet:
-                                            [NSCharacterSet whitespaceCharacterSet]]) {
-                        if (part.length > 0) {
-                            [tokens addObject:part];
-                        }
-                    }
-                    if (tokens.count > 1) {
-                        snprintf(hash_out, hash_out_len, "%s", [tokens[1] UTF8String]);
-                    }
-                    break;
-                }
-            }
-        }
-
-        if (hash_out[0] == '\0') {
-            NSString *msg = [NSString stringWithFormat:
-                             @"created key '%s' but found no hash for it in sc_auth list-ctk-identities",
-                             label];
-            snprintf_error(error_out, error_out_len, msg);
-            return -1;
-        }
-
-        return 0;
-    }
-}
-
-int se_copy_public_key(const char *label,
-                       uint8_t *pub_key_out, int *pub_key_len,
-                       char *error_out, int error_out_len) {
-    @autoreleasepool {
         SecKeyRef privateKey = lookup_ctk_private_key(label, error_out, error_out_len);
         if (!privateKey) {
             return -1;
@@ -174,6 +126,39 @@ int se_copy_public_key(const char *label,
         *pub_key_len = (int)length;
         CFRelease(pubKeyData);
 
+        // Get the identity hash by parsing sc_auth list output
+        hash_out[0] = '\0';
+        NSTask *listTask = [[NSTask alloc] init];
+        listTask.executableURL = [NSURL fileURLWithPath:@"/usr/sbin/sc_auth"];
+        listTask.arguments = @[@"list-ctk-identities"];
+
+        NSPipe *listPipe = [NSPipe pipe];
+        listTask.standardOutput = listPipe;
+        listTask.standardError = [NSPipe pipe];
+
+        if ([listTask launchAndReturnError:&nsError]) {
+            [listTask waitUntilExit];
+            NSData *listData = [listPipe.fileHandleForReading readDataToEndOfFile];
+            NSString *listStr = [[NSString alloc] initWithData:listData
+                                                      encoding:NSUTF8StringEncoding];
+
+            for (NSString *line in [listStr componentsSeparatedByString:@"\n"]) {
+                if ([line containsString:labelStr]) {
+                    NSMutableArray *tokens = [NSMutableArray array];
+                    for (NSString *part in [line componentsSeparatedByCharactersInSet:
+                                            [NSCharacterSet whitespaceCharacterSet]]) {
+                        if (part.length > 0) {
+                            [tokens addObject:part];
+                        }
+                    }
+                    if (tokens.count > 1) {
+                        snprintf(hash_out, hash_out_len, "%s", [tokens[1] UTF8String]);
+                    }
+                    break;
+                }
+            }
+        }
+
         return 0;
     }
 }

internal/secret/atomic.go

@@ -5,15 +5,10 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 
 	"github.com/spf13/afero"
 )
 
-// tempNamePart is in the name of every temporary file WriteFileAtomic makes,
-// ".NAME.tmp-123", and every temporary directory TempDirFor makes, ".tmp-123".
-const tempNamePart = ".tmp-"
-
 // WriteFileAtomic replaces the file at path with data so that a reader, or
 // a crash at any moment, finds either the old content or the new, never a
 // partial file. The data goes into a temporary file that afero.TempFile
@@ -22,7 +17,7 @@ const tempNamePart = ".tmp-"
 // temporary file is removed if any step fails.
 func WriteFileAtomic(fs afero.Fs, path string, data []byte) error {
 	tmp, err := afero.TempFile(fs, filepath.Dir(path),
-		"."+filepath.Base(path)+tempNamePart+"*")
+		"."+filepath.Base(path)+".tmp-*")
 	if err != nil {
 		return fmt.Errorf("failed to create temporary file for %s: %w", path, err)
 	}
@@ -59,7 +54,7 @@ func WriteFileAtomic(fs afero.Fs, path string, data []byte) error {
 // Its name leaves out target's, which may already be as long as a file name
 // can be.
 func TempDirFor(fs afero.Fs, target string) (string, error) {
-	dir, err := afero.TempDir(fs, filepath.Dir(filepath.Dir(target)), tempNamePart)
+	dir, err := afero.TempDir(fs, filepath.Dir(filepath.Dir(target)), ".tmp-")
 	if err != nil {
 		return "", fmt.Errorf(
 			"failed to create temporary directory for %s: %w", target, err)
@@ -68,40 +63,6 @@ func TempDirFor(fs afero.Fs, target string) (string, error) {
 	return dir, nil
 }
 
-// RemoveLeftovers deletes from dir the temporary files of WriteFileAtomic
-// and the temporary directories of TempDirFor that a command killed
-// part-way left there: each entry whose name starts with "." and holds
-// tempNamePart. The caller must hold the state directory lock, so that no
-// running command is still using one. A dir that does not exist holds none.
-func RemoveLeftovers(fs afero.Fs, dir string) error {
-	entries, err := afero.ReadDir(fs, dir)
-	if errors.Is(err, os.ErrNotExist) {
-		return nil
-	}
-
-	if err != nil {
-		return fmt.Errorf("failed to read %s: %w", dir, err)
-	}
-
-	for _, entry := range entries {
-		name := entry.Name()
-		if !strings.HasPrefix(name, ".") || !strings.Contains(name, tempNamePart) {
-			continue
-		}
-
-		path := filepath.Join(dir, name)
-
-		err = fs.RemoveAll(path)
-		if err != nil {
-			return fmt.Errorf("failed to remove %s: %w", path, err)
-		}
-
-		Debug("Removed what an interrupted command left", "path", path)
-	}
-
-	return nil
-}
-
 // WriteDir calls write to write the files of the new directory dir into a
 // temporary directory from TempDirFor, which is then renamed to dir, so that
 // neither a failure nor a crash leaves dir half-written; on a failure the

internal/secret/seunlocker_darwin.go

@@ -217,8 +217,8 @@ func generateSEKeyLabel(vaultName string) (string, error) {
 // using ECIES. No intermediate age keypair is used.
 // The long-term key comes from mnemonic when it is not nil, else from the
 // current unlocker, as getLongTermKeyForSE describes.
-// The SE key is created once the long-term key is in hand and the unlocker's
+// The SE key is created only once everything that does not need it has
-// path is known, and is deleted again if a later step fails.
+// succeeded, and is deleted again if writing the unlocker fails.
 func CreateSecureEnclaveUnlocker(
 	fs afero.Fs,
 	stateDir string,

internal/vault/lock.go

@@ -1,7 +1,6 @@
 package vault
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -15,11 +14,6 @@ import (
 // lockFileName is the file in the state directory that LockStateDir locks.
 const lockFileName = "lock"
 
-// finishedMark is what the lock file holds once the command that last held
-// the lock has released it. A command killed while holding it leaves the
-// file empty.
-const finishedMark = "finished\n"
-
 // memFsLock stands in for the lock file on the in-memory filesystem, which
 // has no file locks. Every in-memory filesystem in the process shares it.
 //
@@ -31,12 +25,6 @@ var memFsLock sync.Mutex
 // it. While one command holds it, the next one waits here. Reads take no
 // lock: each file or directory a command changes is replaced in a single
 // rename, so a reader finds it as it was before or after, never half-made.
-// Once it holds the lock, it empties the lock file, and the function it
-// returns writes finishedMark there just before releasing the lock, so a
-// command killed while holding the lock leaves the mark missing. Finding it
-// missing, LockStateDir first deletes the temporary files and directories
-// such a command may have left, since no command still using them can be
-// running. After a command that finished, it searches nothing.
 //
 // On the real filesystem the lock is flock(2) on the file "lock" in
 // stateDir, which the kernel releases when the process dies, so a killed
@@ -44,97 +32,16 @@ var memFsLock sync.Mutex
 // use has no file locks, so a process-wide mutex stands in for flock there.
 // Any other filesystem is refused rather than left unlocked.
 func LockStateDir(fs afero.Fs, stateDir string) (func(), error) {
-	var release func()
-
 	switch fs.(type) {
 	case *afero.OsFs:
-		var err error
+		return flockStateDir(stateDir)
-
-		release, err = flockStateDir(stateDir)
-		if err != nil {
-			return nil, err
-		}
 	case *afero.MemMapFs:
 		memFsLock.Lock()
 
-		release = memFsLock.Unlock
+		return memFsLock.Unlock, nil
 	default:
 		return nil, fmt.Errorf("%w %T", ErrNoLockForFilesystem, fs)
 	}
-
-	// The lock file is written in place, never replaced: a command waiting
-	// for flock on the old file would then take a lock nobody else checks.
-	lockPath := filepath.Join(stateDir, lockFileName)
-
-	mark, err := afero.ReadFile(fs, lockPath)
-	if err != nil || string(mark) != finishedMark {
-		removeLeftovers(fs, stateDir)
-	}
-
-	err = afero.WriteFile(fs, lockPath, nil, secret.FilePerms)
-	if err != nil {
-		release()
-
-		return nil, fmt.Errorf("failed to empty lock file %s: %w", lockPath, err)
-	}
-
-	return func() {
-		// If this fails, the next command searches when it need not.
-		_ = afero.WriteFile(fs, lockPath, []byte(finishedMark), secret.FilePerms)
-
-		release()
-	}, nil
-}
-
-// removeLeftovers deletes the temporary files and directories that commands
-// killed part-way left in each directory where secret.WriteFileAtomic and
-// secret.TempDirFor make them: the state directory, each vault, each secret
-// and each version. Unlocker directories are written whole by
-// secret.WriteDir and never changed after, so they hold none. A failure is
-// only warned about, and the command goes on.
-func removeLeftovers(fs afero.Fs, stateDir string) {
-	dirs := []string{stateDir}
-
-	for _, vaultDir := range subdirs(fs, filepath.Join(stateDir, "vaults.d")) {
-		dirs = append(dirs, vaultDir)
-
-		for _, secretDir := range subdirs(fs, filepath.Join(vaultDir, "secrets.d")) {
-			dirs = append(dirs, secretDir)
-			dirs = append(dirs, subdirs(fs, filepath.Join(secretDir, "versions"))...)
-		}
-	}
-
-	for _, dir := range dirs {
-		err := secret.RemoveLeftovers(fs, dir)
-		if err != nil {
-			secret.Warn("Failed to remove what an interrupted command left",
-				"error", err)
-		}
-	}
-}
-
-// subdirs returns the directories in dir: none if dir does not exist, and
-// none, with a warning, if it cannot be read.
-func subdirs(fs afero.Fs, dir string) []string {
-	entries, err := afero.ReadDir(fs, dir)
-	if err != nil {
-		if !errors.Is(err, os.ErrNotExist) {
-			secret.Warn("Failed to look for what an interrupted command left",
-				"directory", dir, "error", err)
-		}
-
-		return nil
-	}
-
-	var dirs []string
-
-	for _, entry := range entries {
-		if entry.IsDir() {
-			dirs = append(dirs, filepath.Join(dir, entry.Name()))
-		}
-	}
-
-	return dirs
 }
 
 // flockStateDir takes flock(2) on the lock file in stateDir, creating the

internal/vault/lock_test.go

@@ -1,11 +1,9 @@
 package vault_test
 
 import (
-	"path/filepath"
 	"testing"
 	"time"
 
-	"git.eeqj.de/sneak/secret/internal/secret"
 	"git.eeqj.de/sneak/secret/internal/vault"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
@@ -123,51 +121,6 @@ func TestLockStateDirFreeAfterPanic(t *testing.T) {
 	}
 }
 
-// TestLockStateDirRemovesLeftoversOnlyAfterKill checks that taking the lock
-// deletes a temporary directory a killed command left only when the last
-// holder of the lock did not release it. A holder killed while it holds the
-// lock leaves the lock file as it is at that moment.
-func TestLockStateDirRemovesLeftoversOnlyAfterKill(t *testing.T) {
-	t.Parallel()
-
-	for _, lfs := range lockFilesystems(t) {
-		t.Run(lfs.name, func(t *testing.T) {
-			t.Parallel()
-
-			lockFile := filepath.Join(lfs.stateDir, "lock")
-			leftover := filepath.Join(lfs.stateDir, ".tmp-1")
-
-			release, err := vault.LockStateDir(lfs.fs, lfs.stateDir)
-			require.NoError(t, err)
-
-			whileHeld, err := afero.ReadFile(lfs.fs, lockFile)
-			require.NoError(t, err)
-			release()
-
-			require.NoError(t, lfs.fs.MkdirAll(leftover, secret.DirPerms))
-
-			release, err = vault.LockStateDir(lfs.fs, lfs.stateDir)
-			require.NoError(t, err)
-			release()
-
-			exists, err := afero.DirExists(lfs.fs, leftover)
-			require.NoError(t, err)
-			assert.True(t, exists, "searched after a holder that finished")
-
-			require.NoError(t, afero.WriteFile(lfs.fs, lockFile, whileHeld,
-				secret.FilePerms))
-
-			release, err = vault.LockStateDir(lfs.fs, lfs.stateDir)
-			require.NoError(t, err)
-			release()
-
-			exists, err = afero.DirExists(lfs.fs, leftover)
-			require.NoError(t, err)
-			assert.False(t, exists, "not searched after a holder that was killed")
-		})
-	}
-}
-
 // TestLockStateDirRefusesOtherFilesystems checks that a filesystem with no
 // lock implementation is refused instead of being used unlocked.
 func TestLockStateDirRefusesOtherFilesystems(t *testing.T) {