Files
relicario/docs/superpowers/specs/2026-05-02-v0.5.0-polish-harden-design.md
adlee-was-taken 57237af39e docs(spec): v0.5.0 polish + harden bundle
Anchors on a HIGH-severity auth bypass in the relicario-server
pre-receive hook (revocation + registered-device checks both
unimplemented), bundles two hardening follow-ups, two confirmed
bugs, and four UX improvements. Splits into Plan A (Rust + docs)
and Plan B (extension UX) for independent merge cadence.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-02 15:45:57 -04:00

341 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# v0.5.0 — Polish + Harden — Design
Date: 2026-05-02
Status: Draft
## Overview
v0.5.0 is a "polish + harden" bundle: a security-vulnerability fix, two
hardening follow-ups, two confirmed bugs, and four UX improvements.
No new functional features. Functional work — LastPass import, recovery
QR, Plan 1C-γ, Fullscreen Phases 3/4 — proceeds on parallel plans.
The anchor item is a **HIGH-severity authentication bypass** in
`relicario-server`'s pre-receive hook (S1): both the registered-device
check and the revocation check are unimplemented. Until this lands, the
device-auth system is a no-op.
## Goals
1. Restore device-auth integrity: only registered, non-revoked signing
keys can push to the vault repo.
2. Close two minor hardening gaps surfaced during the security review.
3. Fix two user-visible bugs (strength-meter desync after generate,
raw error codes in the fullscreen tab).
4. Deliver four UX improvements that are polish-shaped, not feature-shaped.
5. Tag v0.5.0 with no known security vulnerabilities or visible-error-code
leaks.
## Scope Map
| Bucket | Item | Plan |
|---|---|---|
| Security | S1: Pre-receive hook fix | A |
| Security | S2: Tar archive path-traversal hardening | A |
| Security | S3: `RELICARIO_*` env-var audit | A |
| Cleanup | C1: Stale feature branch prune | A |
| Bugs | B1: Strength meter desync after regenerate | B |
| Bugs | B2: `vault_locked` raw error code in fullscreen tab | B (subsumed by P4) |
| UX | P1: Password coloring | B |
| UX | P2: Setup-wizard completion → fullscreen vault tab | B |
| UX | P3: Form-layout 2-col → full-width transition | B |
| UX | P4: Error-message audit (snake_case codes → friendly copy) | B |
Two plans split by language/blast-radius:
- **Plan A** = Rust + docs (server, CLI, env-var audit, branch cleanup)
- **Plan B** = Extension UX (TypeScript + CSS)
The plans share no files and can ship independently.
---
## Plan A — Security + Cleanup
### S1. Pre-Receive Hook Fix (anchor)
**Problem.** `crates/relicario-server/src/main.rs:36-81`: `verify_commit`
loads `devices` and `revoked` from the repo at the commit, then drops
both on the floor. It runs `git verify-commit --raw` and accepts on
`GOODSIG` / `Good signature` regardless of which key produced the
signature. This means:
- An attacker who never registered a device can push commits signed
with any GPG key that the server's keyring/allowed-signers happens
to trust (or none at all if the server has no SSH allowed-signers
configured).
- A revoked device's signing key continues to authenticate after
`relicario device revoke` is run — the `revoked.json` write is
cosmetic.
**Fix.** Implement the verification logic per the design spec
(`docs/superpowers/specs/2026-05-02-device-authentication-design.md:284-300`):
1. Build a temporary allowed-signers file from `devices.json` entries
loaded at the commit. Pass it to `git verify-commit` via
`GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=gpg.ssh.allowedSignersFile
GIT_CONFIG_VALUE_0=<tmpfile>` so we don't mutate global git config.
2. Parse the signing-key fingerprint out of `git verify-commit --raw`
output (the fingerprint appears on the `Good "git" signature for …
with … key SHA256:…` line for SSH).
3. Reject if the fingerprint is not in `devices.json` at the commit.
4. Reject if the fingerprint is in `revoked.json` AND
`commit_timestamp >= revoked_at`. The historical-commit case
(timestamp < `revoked_at`) is allowed so old commits survive
revocation.
5. Use the commit's **committer date** (`GIT_COMMITTER_DATE`,
accessible via `git show -s --format=%ct`) — *not* the current wall
clock and not the author date — as `commit_timestamp`. Committer
date is when the signature was applied; author date is when work
was originally written and could be from before revocation even
for a malicious replay.
**Acceptance.**
- An integration test that registers a device, revokes it, signs a
commit with the revoked key dated AFTER `revoked_at`, and confirms
the hook exits non-zero.
- An integration test that signs a commit with a key that was never
registered and confirms the hook exits non-zero.
- An integration test that signs a commit with a registered, non-revoked
key and confirms the hook exits zero.
- An integration test that signs a commit with a revoked key dated
BEFORE `revoked_at` (historical case) and confirms the hook exits zero.
- The dead `let _ = &revoked;` line is gone.
### S2. Tar Archive Path-Traversal Hardening
**Problem.** `crates/relicario-cli/src/main.rs:1722` unpacks the
bundled `git_archive` from a `.relbak` via `tar::Archive::unpack`,
trusting the `tar` crate's defaults. A malicious `.relbak` could
contain entries with `..` components or absolute paths.
**Fix.** Iterate entries explicitly:
- Reject any entry whose path contains a `..` component, an absolute
prefix, or a Windows drive letter.
- Reject symlinks and hardlinks (we never write either inside `.git/`
on restore).
- Cap total uncompressed size at 100× the compressed `git_archive`
size or 1 GiB, whichever is lower (defends against tar bombs).
- Write each entry under `target.join(".git")` only after the path
resolves *inside* that directory.
**Acceptance.**
- Unit test with a hand-crafted tar containing `../../etc/passwd`
restore bails with "path traversal blocked".
- Unit test with a symlink entry — restore bails.
- Unit test with a 1 TiB sparse entry — restore bails on the size cap.
- Existing valid-restore test still passes.
### S3. `RELICARIO_*` Env-Var Audit
**Problem.** Three env vars are not gated by `cfg(debug_assertions)`:
- `RELICARIO_IMAGE` (`crates/relicario-cli/src/session.rs:125`) — reference-image override path.
- `RELICARIO_NO_GROUPS_CACHE` (`crates/relicario-cli/src/helpers.rs:103`) — disables the groups cache.
- `RELICARIO_GITEA_URL` (`crates/relicario-cli/src/main.rs:2298`) — Gitea base URL fallback.
Spot-check confirmed these look intentional, but they're undocumented
and `RELICARIO_NO_GROUPS_CACHE` is more of a dev escape hatch than
production config.
**Fix.**
- Document each env var in `docs/SECURITY.md` and the relevant
`--help` text.
- Move `RELICARIO_NO_GROUPS_CACHE` under `cfg(debug_assertions)`
it's a dev tool, not a user knob.
- Leave `RELICARIO_IMAGE` and `RELICARIO_GITEA_URL` as user-facing
config; audit each call site to confirm no untrusted-input-flows-into-path
cases.
**Acceptance.**
- `docs/SECURITY.md` lists every `RELICARIO_*` env var with purpose
and trust assumption.
- `RELICARIO_NO_GROUPS_CACHE` is no-op in `cargo build --release`.
### C1. Stale Feature Branch Prune
**Problem.** Six local branches with no merged history beyond what's
already in main: `feature/fullscreen-ux-phase-2a`,
`feature/typed-items-1a-rust-core`, `feature/typed-items-1c-alpha`,
`feature/typed-items-1c-beta1`, `feature/typed-items-1c-beta2`.
**Fix.** Verify each is fully merged into main (`git branch --merged
main`), then delete locally only — no remote branch operations. Plan
execution will surface the list and prompt the user before running
`git branch -D` per branch (per project rule on git-destructive ops).
**Acceptance.** `git branch` shows only `main` and active feature
branches.
---
## Plan B — Extension UX
### B1. Strength Meter Desync After Regenerate
**Problem.** Confirmed in screenshot from 2026-05-02: clicking the
regenerate button on a login form's password field rerolls a
20-character mixed-class password (e.g., `sCMtTJkF%GN^mF#-N6D%`) but
the strength meter still reports `~10^1 guesses — trivially crackable`.
The rated string is stale — likely whatever was in the field before
the reroll, or empty.
The meter listens to `input` events on the password field
(`extension/src/shared/form-affordances/password-tools.ts:65`). The
regenerate handler sets `input.value = newPassword` programmatically,
which **does not fire** `input` events in standard DOM behavior.
**Fix.** In the regenerate handler (search for the orange-spinner
button click), after assigning `input.value`, dispatch an
`InputEvent('input', { bubbles: true })`. Confirm by re-rating
inside the handler if needed.
**Acceptance.**
- Vitest: simulate regenerate click, assert that `input` event is
dispatched and meter calls `scheduleRate` with the new value.
- Manual: regenerate a password on the login form, confirm meter
jumps to "strong" with appropriate `guesses_log10`.
### P4. Error-Message Audit (subsumes B2)
**Problem.** Snake_case error codes leak straight into the fullscreen
tab and other surfaces:
- `vault_locked` shown as a red string in the new-login form
(screenshot 2026-05-02).
- ~20 call sites in `extension/src/service-worker/router/` return
`{ ok: false, error: 'vault_locked' }` and similar.
The popup has a one-off mapping at `extension/src/popup/popup.ts:147`
(`/vault_locked/i.test(err)` → unlock prompt), but the fullscreen tab
has no equivalent.
**Fix.**
1. Build a single `ERROR_COPY` map keyed by error code, returning
`{ title: string; body: string; cta?: { label: string; action: () => void } }`.
Place at `extension/src/shared/error-copy.ts`.
2. Audit call sites in `extension/src/service-worker/router/` for all
error codes; ensure each has a mapping.
3. Replace the regex in `popup.ts:147` and the raw-error rendering in
the fullscreen tab with `lookupErrorCopy(code)`.
4. For `vault_locked` specifically: the CTA is "Unlock vault" → opens
the popup unlock view (or in the fullscreen tab, the inline unlock
form).
**Acceptance.**
- No raw `snake_case` strings render in any UI surface for any error
return path. Verified by grep + manual trigger of each error code.
- `vault_locked` in the fullscreen tab shows friendly copy + an
"Unlock vault" button that works.
- Vitest: enumerate every distinct `error: '...'` string literal
found in `extension/src/service-worker/router/` via grep, assert
each is a key in `ERROR_COPY`. Generated test or build-time check
preferred over a static snapshot so it can't drift.
### P1. Password Coloring
**Problem / Fix.** Implement per existing spec at
`docs/superpowers/specs/2026-05-01-password-coloring-design.md` and plan
at `docs/superpowers/plans/2026-05-01-password-coloring.md`. No design
changes; pulled into v0.5.0 because it's polish-flavored and small.
**Acceptance.** Per existing plan's acceptance criteria.
### P2. Setup-Wizard Completion → Fullscreen Vault Tab
**Problem.** Setup wizard currently routes to the popup item-list view
on completion. The user wants to land in the fullscreen vault tab —
the experience reads as more "real software" and avoids the sub-300px
popup width on first run.
**Fix.** In the setup wizard's terminal step, after the vault is
created and the device is registered:
1. Open the fullscreen vault tab via `chrome.tabs.create({ url:
chrome.runtime.getURL('vault.html') })`.
2. Close the setup tab (or the popup, depending on entry point).
**Acceptance.**
- Manual: complete the new-vault setup flow; vault tab opens, setup
tab closes.
- Manual: complete the attach-existing flow; same behavior.
- Vitest: assert `chrome.tabs.create` is called with the vault URL on
successful setup completion.
### P3. Form-Layout 2-col → Full-Width Transition
**Problem.** Screenshot 2026-05-02 shows the new-login form in the
fullscreen tab: the IDENTITY and CREDENTIALS cards sit in a 2-column
grid with a max-width that stops well short of viewport edge, but the
Notes textarea, custom-fields disclosure, and attachments disclosure
below all stretch full-width. The visual rhythm breaks at the
transition — the lower sections look like a different page.
**Fix.** Two candidate approaches; pick at implementation time:
- **A.** Constrain the lower sections to the same max-width and
horizontal alignment as the cards. They become a third "row" in the
same column system. Simpler, less code.
- **B.** Wrap the lower sections in a card matching the upper cards'
visual treatment. Notes-as-card, custom-fields-as-card,
attachments-as-card. More work, more visual consistency, but might
feel heavier.
Recommended: **A** for v0.5.0 — minimal CSS change, immediate
correctness. Revisit B in Phase 3 if the user still feels the layout
is off.
**Acceptance.**
- The new-login form (and all item-type forms — login, secure note,
identity, card, key, document, totp) renders with consistent
horizontal alignment from top of form to bottom.
- Manual: viewport at 1920×1080, 1440×900, 1024×768, and 768×1024;
no jarring width transitions.
---
## Testing Strategy
| Layer | Tests |
|---|---|
| Rust unit (`relicario-server`) | S1 acceptance set (4 scenarios) |
| Rust unit (`relicario-core`) | S2 path-traversal scenarios (3 scenarios) |
| CLI integration | S2 happy-path restore unchanged |
| Vitest (extension) | B1 input-event dispatch; P4 ERROR_COPY snapshot; P2 tabs.create call |
| Manual | P3 viewport sweep; B2/P4 trigger each error code; S1 hook setup on a real Gitea instance; C1 branch cleanup confirmation |
Existing tests stay green. No regression budget.
## Out of Scope
Listed for clarity; each tracked separately:
- LastPass import (`docs/superpowers/plans/2026-04-29-relicario-lastpass-import.md`)
- Recovery QR + entropy floor (`docs/superpowers/plans/2026-05-01-recovery-qr-and-entropy-floor.md`)
- Plan 1C-γ (attachments + Document + trash UI)
- Fullscreen Phase 3 (shell) and Phase 4 (palette)
- Pre-v0.3.0 manual test walk (`docs/test-checklists/2026-04-27-pre-v0.3.0-audit.md`) — that's a v0.3.0 release gate, not a v0.5.0 item
## Sequencing
1. Plan A and Plan B can run in parallel (no shared files).
2. Inside Plan A: S1 first (unblocks the security claim), then S2,
then S3, then C1.
3. Inside Plan B: P4 first (unblocks B2 and surfaces all error codes
centrally), then B1, then P1, then P3, then P2 (P2 is end-to-end
and benefits from earlier polish).
4. Both plans merge to main; tag `v0.5.0` after both PRs are green.
## Risks
- **S1 SSH allowed-signers parsing.** `git verify-commit --raw`
output format for SSH signatures differs slightly from GPG; the
fingerprint-extraction regex needs unit coverage against real
output. Mitigation: capture sample outputs from a test repo and
check them into the test fixtures.
- **P3 layout regressions.** Constraining the lower sections may
affect the textarea behavior at small widths. Mitigation: viewport
sweep in acceptance.
- **C1 accidental deletion.** Plan execution must `git branch
--merged main` filter and prompt before each `-D`. The
no-destructive-without-asking project rule applies.