Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
mergedhumanPR #285Identity

fix(identity): let concurrent runs share a keystore without breaking attestations

Follow-up found while reviewing the merged 00a013b, prioritised by Codex.

Author

JamesCarnley avatar

@JamesCarnley

github profile →
Status
Merged
Merged on
Sep 16
Branch
fix/identity-keystore-concurrency

Description

The pitch.

Follow-up found while reviewing the merged `00a013b`, prioritised by Codex.

### Problem

Runs that share a Town home share its keystore. Every Lab, Track and Path run
calls `attest_bundle`, which creates the operator's controller key on first use.
In a fresh home, concurrent runs raced to create it.

Reproduced on `main` through the CLI, with no test patching: 8 concurrent
`nandatown run voting` runs in each of 10 fresh homes. Of the 80 runs, 26 wrote
attestations that fail `nandatown verify`, and 4 crashed with
`JSONDecodeError`. Three causes:

- **Keys replaced.** `new_identity` checked for a key and then wrote one, so a
  second creator overwrote a key the first was already signing with.
- **Registry torn and lost.** The registry was truncated and rewritten in
  place. A concurrent reader could find it empty, and concurrent writers lost
  each other's entries or added duplicates.
- **Wrong entry chosen.** `identity()` returned the first registry entry with
  the name, which after a race need not match the key on disk. A home damaged
  this way kept producing unverifiable attestations.

### Change

`src/nandatown/identity_portable.py` only. There is no change to file formats,
key or registry locations, or any CLI.

- **Keys are never replaced.** A new key is created under an exclusive lock on
  the keystore (`flock`, or `msvcrt` on Windows), which the system releases if
  the process dies. It is written whole under a staging name and then
  hard-linked into place, which fails rather than replace a key already there.
  Without hard links, the path is first claimed with an exclusive create and
  the whole key then replaces that claim. Either way no key is replaced,
  whether or not any writer holds the lock, and a reader sees an empty claim
  (which it waits for) or a whole key, never part of one.
- **The registry is written whole.** It is updated under the same lock and
  replaced atomically, so readers never see it half written. It keeps its mode,
  and a symlinked registry is updated where the link points. In that case the
  lock of the directory it really lives in is taken too, so homes sharing one
  registry don't lose entries. Directories are identified and ordered by device
  and inode, so one directory spelled two ways, as on a case-insensitive disk,
  is never locked twice.
- **The key on disk decides,** provided the registry lists it under that name.
  So a home the race already damaged attests verifiably again, and no entry is
  removed. A key the registry lists under another name, or not at all, is an
  `IdentityError`, as on `main`.
- **Recovery:**
  - A key left without its registry entry, by a process stopped between the
    two writes, is registered on next use.
  - A staged private key left by a killed writer is deleted when a later
    process takes the lock, once it is ten minutes old. Only regular files
    named exactly as Town stages them are removed.
  - An empty key left by a writer killed after claiming it is reported with
    how to recover.
- **Fallbacks:**
  - **No hard links:** replace, under the lock, only when no key exists.
  - **Locks unavailable** (e.g. NFS without a lock daemon, or a shared
    registry directory whose lock this user cannot open): keys are still never
    replaced, but concurrent writers can lose registry entries, and Town
    warns.

### Verification

- **CLI reproduction, same 80 runs:** all verify, with one registry entry per
  home. On `main`, 5 of the 10 homes had bad runs.
- **Regression tests.** They start real processes with widened timing windows,
  so they don't depend on luck. They cover:
  - concurrent runs attesting verifiably;
  - established keys and other names' entries preserved;
  - concurrent new names all registered;
  - a home damaged by the old race;
  - a key that arrives between the check and the link, with and without hard
    links;
  - a leftover staged key;
  - keys listed under another name or not at all, and damaged keys;
  - a symlinked registr

…

Try it

Open PR on GitHubView diff

Checkout locally

git fetch origin pull/285/head:pr-285
git checkout pr-285