a73x

specs/003-key-trust-allowlist/plan.md

Ref:   Size: 6.4 KiB

# Implementation Plan: Key Trust Allowlist

**Branch**: `003-key-trust-allowlist` | **Date**: 2026-03-21 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `/specs/003-key-trust-allowlist/spec.md`

## Summary

Add a trusted-keys allowlist that gates sync verification. Currently `verify_ref()` accepts any valid Ed25519 signature. This feature adds a project-local trusted keys file (`.git/collab/trusted-keys`) and CLI commands (`collab key add/list/remove`) so that only events signed by explicitly trusted public keys are accepted during sync. When no trusted keys file exists, the system falls back to current behavior with a warning.

## Technical Context

**Language/Version**: Rust 2021 edition
**Primary Dependencies**: git2 0.19, clap 4 (derive), ed25519-dalek 2, base64 0.22, serde/serde_json 1, dirs 5, thiserror 2
**Storage**: Git refs under `.git/refs/collab/`, trusted keys file at `.git/collab/trusted-keys` (plain text, not a git object)
**Testing**: `cargo test`, integration tests in `tests/`, unit tests inline (`#[cfg(test)]`), tempfile 3 for test repos
**Target Platform**: Linux/macOS CLI
**Project Type**: CLI tool (binary crate)
**Performance Goals**: Key management operations complete in under 1 second (SC-004)
**Constraints**: No new dependencies required. All functionality covered by existing crates.
**Scale/Scope**: Trusted keys files expected to have <100 entries per project.

## Constitution Check

Constitution is unconfigured (template placeholders). No gates to enforce.

## Architecture

### Integration with Existing verify_ref Flow

The key trust check is a **post-verification filter** layered on top of the existing `verify_ref()` function. The flow becomes:

1. `sync::reconcile_refs()` calls `signing::verify_ref()` (unchanged -- still checks cryptographic validity)
2. After `verify_ref()` returns, `reconcile_refs()` calls a new `trust::check_trust()` function that:
   - Loads the trusted keys file (if it exists)
   - For each `VerifyStatus::Valid` result, checks whether `result.pubkey` is in the trusted set
   - Returns a new `VerifyStatus::Untrusted` variant (or an `UntrustedKey` error) for keys not in the set
3. If no trusted keys file exists, all valid signatures pass through with a warning

This design keeps `verify_ref()` purely about cryptographic correctness and separates policy (trust) into its own module.

### New VerifyStatus Variant

Add `Untrusted` to the `VerifyStatus` enum:

```rust
pub enum VerifyStatus {
    Valid,
    Invalid,
    Missing,
    Untrusted,  // new: signature valid but key not in trusted set
}
```

### Trusted Keys File Format

Location: `.git/collab/trusted-keys`

```
# Comments start with #
<base64-pubkey> <optional label with spaces>
<base64-pubkey>
```

First space delimits key from label. Lines starting with `#` are comments. Empty lines skipped. Malformed lines produce a warning and are skipped.

### CLI Integration

New top-level `Key` subcommand group in `cli.rs`:

```
collab key add <pubkey> [--label <label>] [--self]
collab key list
collab key remove <pubkey>
```

`--self` reads from `~/.config/git-collab/signing-key.pub` and adds it.

## Project Structure

### Documentation (this feature)

```text
specs/003-key-trust-allowlist/
├── plan.md              # This file
├── spec.md              # Feature specification
├── research.md          # Codebase analysis and design research
├── data-model.md        # Trusted keys file format specification
├── quickstart.md        # Developer quickstart for implementation
├── contracts/
│   └── cli-commands.md  # CLI command contracts
└── checklists/          # Acceptance checklists
```

### Source Code (repository root)

```text
src/
├── trust.rs             # NEW: trusted keys file I/O, trust checking logic
├── signing.rs           # MODIFIED: add Untrusted variant to VerifyStatus
├── sync.rs              # MODIFIED: integrate trust check into reconcile_refs
├── cli.rs               # MODIFIED: add Key subcommand group
├── lib.rs               # MODIFIED: add trust module, wire Key commands
├── error.rs             # MODIFIED: add UntrustedKey error variant

tests/
├── trust_test.rs        # NEW: unit/integration tests for trust module
└── common/              # Shared test helpers (existing)
```

**Structure Decision**: Single flat module (`src/trust.rs`) following the existing pattern of one module per domain concept (signing, sync, issue, patch). No new directories needed.

## Key Design Decisions

### 1. Separate trust module vs. extending signing.rs

**Decision**: New `src/trust.rs` module.
**Rationale**: `signing.rs` handles cryptographic operations (sign, verify). Trust policy (which keys to accept) is a distinct concern. Separation keeps both modules focused and testable independently.

### 2. Untrusted as a VerifyStatus variant vs. separate error

**Decision**: Add `Untrusted` variant to `VerifyStatus`.
**Rationale**: The sync reconciliation loop already pattern-matches on `VerifyStatus`. Adding a variant keeps the rejection logic uniform rather than requiring a second error-handling path.

### 3. Trust check location: verify_ref vs. reconcile_refs

**Decision**: Check trust in `reconcile_refs()` after `verify_ref()` returns, not inside `verify_ref()`.
**Rationale**: `verify_ref()` is a pure cryptographic function that takes only `(repo, ref_name)`. Adding trust would require passing the trusted keys set and coupling it to file I/O. Keeping trust checking in `reconcile_refs()` (which already has access to the repo workdir) is cleaner.

### 4. Fallback behavior when no trusted keys file exists

**Decision**: Accept all valid signatures, print a warning.
**Rationale**: Per FR-007 and spec edge cases. This ensures backward compatibility and a smooth adoption path.

### 5. Key validation on add

**Decision**: Validate base64 decoding AND 32-byte length AND Ed25519 point validity on `collab key add`.
**Rationale**: Per FR-008. Catching invalid keys early prevents confusing errors during sync. Reuse existing `VerifyingKey::from_bytes()` from ed25519-dalek.

### 6. File locking

**Decision**: No file locking for trusted-keys file.
**Rationale**: The file is local-only, single-user. Concurrent CLI invocations on the same file are unlikely and the worst case is a benign race. Consistent with how git itself handles similar local config files.

## Complexity Tracking

No constitution violations to justify.